Remove redundant checks and reduce dependencies for generating swap m…

…essages (#758)

smart-contracts/osmosis/contracts/cl-vault/src/contract.rs

@@ -138,7 +138,7 @@ pub fn execute(
                     )?,
                 ),
                 crate::msg::ExtensionExecuteMsg::SwapNonVaultFunds { swap_operations } => {
-                    execute_swap_non_vault_funds(deps, env, info, swap_operations)
+                    execute_swap_non_vault_funds(deps, env.contract.address, info, swap_operations)
                 }
                 crate::msg::ExtensionExecuteMsg::CollectRewards {} => {
                     execute_collect_rewards(deps, env)

smart-contracts/osmosis/contracts/cl-vault/src/error.rs

@@ -59,12 +59,6 @@ pub enum ContractError {
         actual: Vec<Coin>,
     },
 
-    #[error("Bad token out requested for swap, must be one of: {base_token:?}, {quote_token:?}")]
-    BadTokenForSwap {
-        base_token: String,
-        quote_token: String,
-    },
-
     #[error("Insufficient funds for swap. Have: {balance}, Need: {needed}")]
     InsufficientFundsForSwap { balance: Uint128, needed: Uint128 },
 

smart-contracts/osmosis/contracts/cl-vault/src/helpers/msgs.rs

@@ -1,6 +1,5 @@
 use cosmwasm_std::{
-    attr, to_json_binary, Addr, Attribute, BankMsg, Coin, CosmosMsg, Deps, DepsMut, Env, Uint128,
-    WasmMsg,
+    attr, to_json_binary, Addr, Attribute, BankMsg, Coin, CosmosMsg, Deps, Env, Uint128, WasmMsg,
 };
 use dex_router_osmosis::msg::ExecuteMsg as DexRouterExecuteMsg;
 use osmosis_std::types::{
@@ -11,11 +10,7 @@ use osmosis_std::types::{
     },
 };
 
-use crate::{
-    state::{DEX_ROUTER, POSITION},
-    vault::swap::SwapParams,
-    ContractError,
-};
+use crate::{state::POSITION, vault::swap::SwapParams, ContractError};
 
 // Bank
 
@@ -55,50 +50,45 @@ pub fn refund_bank_msg(
 }
 
 /// Swaps
-
-/// swap will always swap over the CL pool. In the future we may expand the
-/// feature such that it chooses best swaps over all routes
-pub fn swap_msg(deps: &DepsMut, env: &Env, params: SwapParams) -> Result<CosmosMsg, ContractError> {
-    // let pool_config = POOL_CONFIG.load(deps.storage)?;
-    let dex_router = DEX_ROUTER.may_load(deps.storage)?;
-
-    // we will only ever have a route length of one, this will likely change once we start selecting different routes
+pub fn swap_msg(
+    sender: Addr,
+    params: SwapParams,
+    dex_router: Option<Addr>,
+) -> Result<CosmosMsg, ContractError> {
     let pool_route = SwapAmountInRoute {
         pool_id: params.pool_id,
         token_out_denom: params.token_out_denom.to_string(),
     };
 
-    // if we don't have a dex_router, we will always swap over the osmosis pool
-    if dex_router.is_none() {
-        return Ok(osmosis_swap_exact_amount_in_msg(
-            env,
+    if let Some(dex_router) = dex_router {
+        cw_dex_execute_swap_operations_msg(
+            dex_router,
+            params.forced_swap_route,
+            params.token_in_denom.to_string(),
+            params.token_in_amount,
+            params.token_out_denom.to_string(),
+            params.token_out_min_amount,
+        )
+    } else {
+        Ok(osmosis_swap_exact_amount_in_msg(
+            sender,
             pool_route,
             params.token_in_amount,
             &params.token_in_denom.to_string(),
             params.token_out_min_amount,
-        ));
+        ))
     }
-
-    // we know we have a dex_router, so we can unwrap it and execute the swap
-    cw_dex_execute_swap_operations_msg(
-        dex_router.clone().unwrap(),
-        params.forced_swap_route,
-        params.token_in_denom.to_string(),
-        params.token_in_amount,
-        params.token_out_denom.to_string(),
-        params.token_out_min_amount,
-    )
 }
 
 fn osmosis_swap_exact_amount_in_msg(
-    env: &Env,
+    sender: Addr,
     pool_route: SwapAmountInRoute,
     token_in_amount: Uint128,
     token_in_denom: &String,
     token_out_min_amount: Uint128,
 ) -> CosmosMsg {
     osmosis_std::types::osmosis::poolmanager::v1beta1::MsgSwapExactAmountIn {
-        sender: env.contract.address.to_string(),
+        sender: sender.to_string(),
         routes: vec![pool_route],
         token_in: Some(OsmoCoin {
             denom: token_in_denom.to_string(),

smart-contracts/osmosis/contracts/cl-vault/src/vault/deposit.rs

@@ -2,13 +2,13 @@ use crate::{
     helpers::{
         getters::{
             get_asset0_value, get_depositable_tokens, get_single_sided_deposit_0_to_1_swap_amount,
-            get_single_sided_deposit_1_to_0_swap_amount,
+            get_single_sided_deposit_1_to_0_swap_amount, get_twap_price,
         },
         msgs::refund_bank_msg,
     },
     query::{query_total_assets, query_total_vault_token_supply},
     reply::Replies,
-    state::{CURRENT_SWAP_ANY_DEPOSIT, POOL_CONFIG, SHARES, VAULT_DENOM},
+    state::{CURRENT_SWAP_ANY_DEPOSIT, DEX_ROUTER, POOL_CONFIG, SHARES, VAULT_DENOM},
     vault::{
         concentrated_liquidity::{get_cl_pool_info, get_position},
         swap::{calculate_swap_amount, SwapDirection},
@@ -118,15 +118,18 @@ pub(crate) fn execute_any_deposit(
             (deposit_info.base_deposit, deposit_info.quote_deposit),
         ),
     )?;
+
+    let dex_router = DEX_ROUTER.may_load(deps.storage)?;
+    let twap_price = get_twap_price(deps.storage, &deps.querier, &env, 24u64)?;
     let swap_calc_result = calculate_swap_amount(
-        deps,
-        &env,
+        env.contract.address,
         pool_config,
         swap_direction,
         swap_amount,
         max_slippage,
         None, // TODO: check this None
-        24u64,
+        twap_price,
+        dex_router,
     )?;
 
     // rest minting logic remains same

smart-contracts/osmosis/contracts/cl-vault/src/vault/range.rs

@@ -3,15 +3,15 @@ use crate::{
         assert::assert_range_admin,
         getters::{
             get_single_sided_deposit_0_to_1_swap_amount,
-            get_single_sided_deposit_1_to_0_swap_amount, get_unused_pair_balances,
+            get_single_sided_deposit_1_to_0_swap_amount, get_twap_price, get_unused_pair_balances,
         },
     },
     math::tick::price_to_tick,
     msg::{ExecuteMsg, MergePositionMsg},
     reply::Replies,
     state::{
-        ModifyRangeState, Position, SwapDepositMergeState, MODIFY_RANGE_STATE, POOL_CONFIG,
-        POSITION, SWAP_DEPOSIT_MERGE_STATE,
+        ModifyRangeState, Position, SwapDepositMergeState, DEX_ROUTER, MODIFY_RANGE_STATE,
+        POOL_CONFIG, POSITION, SWAP_DEPOSIT_MERGE_STATE,
     },
     vault::{
         concentrated_liquidity::{create_position, get_cl_pool_info, get_position},
@@ -253,7 +253,7 @@ pub fn handle_initial_create_position_reply(
 ///
 /// It also calculates the exact amount we should be swapping based on current balances and the new range
 #[allow(clippy::too_many_arguments)]
-pub fn do_swap_deposit_merge(
+fn do_swap_deposit_merge(
     deps: DepsMut,
     env: Env,
     target_lower_tick: i64,
@@ -306,15 +306,18 @@ pub fn do_swap_deposit_merge(
             SwapDirection::OneToZero,
         )
     };
+
+    let dex_router = DEX_ROUTER.may_load(deps.storage)?;
+    let twap_price = get_twap_price(deps.storage, &deps.querier, &env, twap_window_seconds)?;
     let swap_calc_result = calculate_swap_amount(
-        deps,
-        &env,
+        env.contract.address,
         pool_config,
         swap_direction,
         token_in_amount,
         mrs.max_slippage,
         mrs.forced_swap_route,
-        twap_window_seconds,
+        twap_price,
+        dex_router,
     )?;
 
     Ok(Response::new()

smart-contracts/osmosis/contracts/cl-vault/src/vault/swap.rs

@@ -1,10 +1,10 @@
-use cosmwasm_std::{CosmosMsg, Decimal, DepsMut, Env, Fraction, MessageInfo, Response, Uint128};
+use cosmwasm_std::{Addr, CosmosMsg, Decimal, DepsMut, MessageInfo, Response, Uint128};
 use osmosis_std::types::osmosis::poolmanager::v1beta1::SwapAmountInRoute;
 
 use crate::{
-    helpers::{assert::assert_range_admin, getters::get_twap_price, msgs::swap_msg},
+    helpers::{assert::assert_range_admin, msgs::swap_msg},
     msg::SwapOperation,
-    state::{PoolConfig, POOL_CONFIG, VAULT_CONFIG},
+    state::{PoolConfig, DEX_ROUTER, POOL_CONFIG, VAULT_CONFIG},
     ContractError,
 };
 
@@ -34,7 +34,7 @@ pub struct SwapParams {
 
 pub fn execute_swap_non_vault_funds(
     deps: DepsMut,
-    env: Env,
+    contract_address: Addr,
     info: MessageInfo,
     swap_operations: Vec<SwapOperation>,
 ) -> Result<Response, ContractError> {
@@ -64,7 +64,7 @@ pub fn execute_swap_non_vault_funds(
         let balance_in_contract = deps
             .querier
             .query_balance(
-                env.clone().contract.address,
+                contract_address.clone(),
                 swap_operation.clone().token_in_denom,
             )?
             .amount;
@@ -82,18 +82,14 @@ pub fn execute_swap_non_vault_funds(
             .checked_div(Uint128::new(2))?;
 
         // TODO_FUTURE: We should be passing the max_slippage from outside as we do during ModifyRange
-        let token_out_min_amount_0 = part_0_amount.checked_multiply_ratio(
-            vault_config.swap_max_slippage.numerator(),
-            vault_config.swap_max_slippage.denominator(),
-        )?;
-        let token_out_min_amount_1 = part_1_amount.checked_multiply_ratio(
-            vault_config.swap_max_slippage.numerator(),
-            vault_config.swap_max_slippage.denominator(),
-        )?;
+        let token_out_min_amount_0 =
+            part_0_amount.checked_mul_floor(vault_config.swap_max_slippage)?;
+        let token_out_min_amount_1 =
+            part_1_amount.checked_mul_floor(vault_config.swap_max_slippage)?;
 
+        let dex_router = DEX_ROUTER.may_load(deps.storage)?;
         swap_msgs.push(swap_msg(
-            &deps,
-            &env,
+            contract_address.clone(),
             SwapParams {
                 pool_id: swap_operation.pool_id_0,
                 token_in_amount: part_0_amount,
@@ -102,10 +98,10 @@ pub fn execute_swap_non_vault_funds(
                 token_out_denom: pool_token_0,
                 forced_swap_route: swap_operation.forced_swap_route_token_0,
             },
+            dex_router.clone(),
         )?);
         swap_msgs.push(swap_msg(
-            &deps,
-            &env,
+            contract_address.clone(),
             SwapParams {
                 pool_id: swap_operation.pool_id_1,
                 token_in_amount: part_1_amount,
@@ -114,6 +110,7 @@ pub fn execute_swap_non_vault_funds(
                 token_out_denom: pool_token_1,
                 forced_swap_route: swap_operation.forced_swap_route_token_1,
             },
+            dex_router,
         )?);
     }
 
@@ -125,46 +122,31 @@ pub fn execute_swap_non_vault_funds(
 
 #[allow(clippy::too_many_arguments)]
 pub fn calculate_swap_amount(
-    deps: DepsMut,
-    env: &Env,
+    contract_address: Addr,
     pool_config: PoolConfig,
     swap_direction: SwapDirection,
     token_in_amount: Uint128,
     max_slippage: Decimal,
     forced_swap_route: Option<Vec<SwapAmountInRoute>>,
-    twap_window_seconds: u64,
+    twap_price: Decimal,
+    dex_router: Option<Addr>,
 ) -> Result<SwapCalculationResult, ContractError> {
-    let twap_price = get_twap_price(deps.storage, &deps.querier, env, twap_window_seconds)?;
     let (token_in_denom, token_out_denom, token_out_ideal_amount) = match swap_direction {
         SwapDirection::ZeroToOne => (
             &pool_config.token0,
             &pool_config.token1,
-            token_in_amount
-                .checked_multiply_ratio(twap_price.numerator(), twap_price.denominator()),
+            token_in_amount.checked_mul_floor(twap_price),
         ),
         SwapDirection::OneToZero => (
             &pool_config.token1,
             &pool_config.token0,
-            token_in_amount
-                .checked_multiply_ratio(twap_price.denominator(), twap_price.numerator()),
+            token_in_amount.checked_div_floor(twap_price),
         ),
     };
 
-    let token_out_min_amount = token_out_ideal_amount?
-        .checked_multiply_ratio(max_slippage.numerator(), max_slippage.denominator())?;
-
-    if !pool_config.pool_contains_token(token_in_denom) {
-        return Err(ContractError::BadTokenForSwap {
-            base_token: pool_config.token0,
-            quote_token: pool_config.token1,
-        });
-    }
-
-    // generate a swap message with recommended path as the current
-    // pool on which the vault is running
+    let token_out_min_amount = token_out_ideal_amount?.checked_mul_floor(max_slippage)?;
     let swap_msg = swap_msg(
-        &deps,
-        env,
+        contract_address,
         SwapParams {
             pool_id: pool_config.pool_id,
             token_in_amount,
@@ -173,6 +155,7 @@ pub fn calculate_swap_amount(
             token_out_denom: token_out_denom.clone(),
             forced_swap_route,
         },
+        dex_router,
     )?;
 
     Ok(SwapCalculationResult {
@@ -228,7 +211,7 @@ mod tests {
             forced_swap_route: None,
         };
 
-        let result = super::swap_msg(&deps_mut, &env, swap_params).unwrap();
+        let result = super::swap_msg(env.contract.address.clone(), swap_params, None).unwrap();
 
         if let CosmosMsg::Stargate { type_url: _, value } = result {
             let msg_swap =