From 5ee39984aa21730a14437ece25085ae61197d2bd Mon Sep 17 00:00:00 2001 From: Sturdy <91910406+apollo-sturdy@users.noreply.github.com> Date: Mon, 25 Sep 2023 21:46:27 +0200 Subject: [PATCH] feat: Enforce min_out locally for withdrawals --- src/contract.rs | 24 +----- src/deposit.rs | 78 ++++++++++++------ src/msg.rs | 19 +++-- src/state.rs | 1 - src/withdraw.rs | 177 ++++++++++++++++++++------------------- tests/test_withdraw.rs | 182 ++++++++++++++++++++++++++++++++++++++--- 6 files changed, 324 insertions(+), 157 deletions(-) diff --git a/src/contract.rs b/src/contract.rs index d4b3d9b..22c8eeb 100644 --- a/src/contract.rs +++ b/src/contract.rs @@ -142,19 +142,11 @@ pub fn execute( deposit_asset_info, ), CallbackMsg::EnforceMinOut { - asset, - recipient, - balance_before, - min_out, - } => callback_enforce_min_out( - deps, - env, - info, - asset, + assets, recipient, - balance_before, + balances_before, min_out, - ), + } => callback_enforce_min_out(deps, assets, recipient, balances_before, min_out), CallbackMsg::AfterRedeem { receive_choice, vault_base_token, @@ -172,15 +164,7 @@ pub fn execute( assets, receive_choice, recipient, - min_out, - } => callback_after_withdraw_liq( - deps, - env, - assets, - receive_choice, - recipient, - min_out, - ), + } => callback_after_withdraw_liq(deps, env, assets, receive_choice, recipient), } } } diff --git a/src/deposit.rs b/src/deposit.rs index 3a9bbe0..596dfc2 100644 --- a/src/deposit.rs +++ b/src/deposit.rs @@ -1,7 +1,7 @@ -use apollo_cw_asset::{AssetInfo, AssetList}; +use apollo_cw_asset::{Asset, AssetInfo, AssetList}; use apollo_utils::assets::receive_assets; use cosmwasm_std::{ - to_binary, Addr, Binary, Coin, DepsMut, Empty, Env, MessageInfo, Response, Uint128, + to_binary, Addr, Binary, Coin, DepsMut, Empty, Env, Event, MessageInfo, Response, Uint128, }; use cw_dex::traits::Pool as PoolTrait; use cw_dex::Pool; @@ -16,7 +16,7 @@ pub fn execute_deposit( deps: DepsMut, env: Env, info: MessageInfo, - caller_funds: AssetList, + assets: AssetList, vault_address: Addr, recipient: Option, min_out: Uint128, @@ -24,7 +24,7 @@ pub fn execute_deposit( // Unwrap recipient or use sender let recipient = recipient.map_or(Ok(info.sender.clone()), |x| deps.api.addr_validate(&x))?; - let receive_assets_res = receive_assets(&info, &env, &caller_funds)?; + let receive_assets_res = receive_assets(&info, &env, &assets)?; // Query the vault info to get the deposit asset let vault: VaultContract = VaultContract::new(&deps.querier, &vault_address)?; @@ -37,25 +37,33 @@ pub fn execute_deposit( let vault_token = AssetInfo::native(&vault.vault_token); let balance_before = vault_token.query_balance(&deps.querier, recipient.clone())?; let enforce_min_out_msg = CallbackMsg::EnforceMinOut { - asset: vault_token, + assets: vec![vault_token.clone()], recipient: recipient.clone(), - balance_before, - min_out, + balances_before: vec![Asset::new(vault_token.clone(), balance_before)].into(), + min_out: vec![Asset::new(vault_token.clone(), min_out)].into(), } .into_cosmos_msg(&env)?; + let event = Event::new("apollo/vault-zapper/execute_deposit") + .add_attribute("assets", assets.to_string()) + .add_attribute("vault_address", &vault_address) + .add_attribute("recipient", &recipient) + .add_attribute("min_out", min_out); + // Check if coins sent are already same as the depositable assets // If yes, then just deposit the coins - if caller_funds.len() == 1 && caller_funds.to_vec()[0].info == deposit_asset_info { - let amount = caller_funds.to_vec()[0].amount; + if assets.len() == 1 && assets.to_vec()[0].info == deposit_asset_info { + let amount = assets.to_vec()[0].amount; let msgs = vault.increase_allowance_and_deposit( amount, &deposit_asset_info, Some(recipient.to_string()), )?; + return Ok(receive_assets_res .add_messages(msgs) - .add_message(enforce_min_out_msg)); + .add_message(enforce_min_out_msg) + .add_event(event)); } //Check if the depositable asset is an LP token @@ -80,7 +88,7 @@ pub fn execute_deposit( // Basket Liquidate deposited coins // We only liquidate the coins that are not already the target asset - let liquidate_coins = caller_funds + let liquidate_coins = assets .into_iter() .filter_map(|a| { if !receive_asset_infos.contains(&a.info) { @@ -125,7 +133,8 @@ pub fn execute_deposit( Ok(receive_assets_res .add_messages(msgs) - .add_message(enforce_min_out_msg)) + .add_message(enforce_min_out_msg) + .add_event(event)) } pub fn callback_provide_liquidity( @@ -198,22 +207,39 @@ pub fn callback_deposit( pub fn callback_enforce_min_out( deps: DepsMut, - _env: Env, - _info: MessageInfo, - asset: AssetInfo, + assets: Vec, recipient: Addr, - balance_before: Uint128, - min_out: Uint128, + balances_before: AssetList, + min_out: AssetList, ) -> Result { - let new_balance = asset.query_balance(&deps.querier, recipient)?; - let diff = new_balance.saturating_sub(balance_before); - - if diff < min_out { - return Err(ContractError::MinOutNotMet { - min_out, - actual: diff, - }); + let mut new_balances = + AssetList::query_asset_info_balances(assets.clone(), &deps.querier, &recipient)?; + let assets_received = new_balances.deduct_many(&balances_before)?; + + for asset in min_out.iter() { + let received = assets_received + .find(&asset.info) + .map(|x| x.amount) + .unwrap_or_default(); + if received < asset.amount { + return Err(ContractError::MinOutNotMet { + min_out: asset.amount, + actual: received, + }); + } + } + + let mut event = Event::new("apollo/vault-zapper/callback_enforce_min_out") + .add_attribute("recipient", recipient); + if !assets.is_empty() { + event = event.add_attribute("assets", format!("{:?}", assets)); + } + if min_out.len() > 0 { + event = event.add_attribute("min_out", format!("{:?}", min_out)); + } + if assets_received.len() > 0 { + event = event.add_attribute("assets_received", format!("{:?}", assets_received)); } - Ok(Response::new()) + Ok(Response::new().add_event(event)) } diff --git a/src/msg.rs b/src/msg.rs index 1cd214c..394295b 100644 --- a/src/msg.rs +++ b/src/msg.rs @@ -81,17 +81,19 @@ pub enum CallbackMsg { recipient: Addr, deposit_asset_info: AssetInfo, }, - /// Enforce that the minimum amount of vault tokens are received + /// Enforce that the minimum amount of the specified assets are sent to the + /// recipient after the transaction EnforceMinOut { - /// The asset to check the balance of - asset: AssetInfo, + /// The assets to check the balance of + assets: Vec, /// The address to check the balance of recipient: Addr, - /// The recipient's balance of `asset` before the transaction - balance_before: Uint128, - /// The minimum amount of `asset` to receive. If the amount of - /// `asset` received is less than this, the transaction will fail. - min_out: Uint128, + /// The recipient's balance of each of the assets before the transaction + balances_before: AssetList, + /// The minimum amount of each asset to receive. If the amount received + /// of any of the assets is less than this, the transaction will + /// fail. + min_out: AssetList, }, /// Called after redeeming vault tokens AfterRedeem { @@ -105,7 +107,6 @@ pub enum CallbackMsg { assets: Vec, receive_choice: ReceiveChoice, recipient: Addr, - min_out: AssetList, }, } diff --git a/src/state.rs b/src/state.rs index 674c6b8..2b2d37b 100644 --- a/src/state.rs +++ b/src/state.rs @@ -299,7 +299,6 @@ mod tests { .unwrap() .map(|x| x.unwrap().0) .collect(); - println!("{:?}", res); assert_eq!(res.len(), 3); assert_eq!( res, diff --git a/src/withdraw.rs b/src/withdraw.rs index 2764ae9..6e1a931 100644 --- a/src/withdraw.rs +++ b/src/withdraw.rs @@ -1,6 +1,7 @@ use apollo_cw_asset::{Asset, AssetInfo, AssetList}; +use cosmwasm_schema::cw_serde; use cosmwasm_std::{ - to_binary, Addr, CosmosMsg, DepsMut, Empty, Env, MessageInfo, Response, Uint128, WasmMsg, + to_binary, Addr, CosmosMsg, DepsMut, Empty, Env, Event, MessageInfo, Response, Uint128, WasmMsg, }; use cw_dex::traits::Pool as PoolTrait; use cw_dex::Pool; @@ -12,6 +13,7 @@ use crate::msg::{CallbackMsg, ReceiveChoice}; use crate::state::{LOCKUP_IDS, ROUTER}; use crate::ContractError; +#[cw_serde] pub enum RedeemType { Normal, Lockup(u64), @@ -117,15 +119,27 @@ pub fn withdraw( }), }; - Ok(Response::new().add_message(withdraw_msg).add_message( - CallbackMsg::AfterRedeem { - receive_choice, - vault_base_token, - recipient, - min_out, - } - .into_cosmos_msg(&env)?, - )) + let mut event = Event::new("apollo/vault-zapper/withdraw") + .add_attribute("vault_address", &vault_address) + .add_attribute("recipient", &recipient) + .add_attribute("receive_choice", format!("{:?}", receive_choice)) + .add_attribute("withdraw_type", format!("{:?}", withdraw_type)); + if min_out.len() > 0 { + event = event.add_attribute("min_out", min_out.to_string()); + } + + Ok(Response::new() + .add_message(withdraw_msg) + .add_message( + CallbackMsg::AfterRedeem { + receive_choice, + vault_base_token, + recipient, + min_out, + } + .into_cosmos_msg(&env)?, + ) + .add_event(event)) } pub fn callback_after_redeem( @@ -144,65 +158,88 @@ pub fn callback_after_redeem( let pool = Pool::get_pool_for_lp_token(deps.as_ref(), &vault_base_token).ok(); // Check requested withdrawal assets - match &receive_choice { + let (res, withdrawal_assets) = match &receive_choice { ReceiveChoice::SwapTo(requested_asset) => { // If the requested denom is the same as the vaults withdrawal asset, just send // it to the recipient. if requested_asset == &vault_base_token { - return Ok(Response::new().add_message(base_token.transfer_msg(recipient)?)); - } - - // Check if the withdrawable asset is an LP token. - let router = ROUTER.load(deps.storage)?; - - if let Some(pool) = pool { - // Add messages to withdraw liquidity - let withdraw_liq_res = - pool.withdraw_liquidity(deps.as_ref(), &env, base_token, AssetList::new())?; - Ok(withdraw_liq_res.add_message( - CallbackMsg::AfterWithdrawLiq { - assets: pool.pool_assets(deps.as_ref())?, - receive_choice, - recipient, - min_out, - } - .into_cosmos_msg(&env)?, + Ok(( + Response::new().add_message(base_token.transfer_msg(&recipient)?), + vec![base_token.info], )) } else { - // Basket liquidate the asset withdrawn from the vault - let min_out = unwrap_min_out(min_out, requested_asset)?; - let msgs = router.basket_liquidate_msgs( - vec![base_token].into(), - requested_asset, - Some(min_out), - Some(recipient.to_string()), - )?; - Ok(Response::new().add_messages(msgs)) + // Check if the withdrawable asset is an LP token. + let router = ROUTER.load(deps.storage)?; + + if let Some(pool) = pool { + // Add messages to withdraw liquidity + let withdraw_liq_res = + pool.withdraw_liquidity(deps.as_ref(), &env, base_token, AssetList::new())?; + Ok(( + withdraw_liq_res.add_message( + CallbackMsg::AfterWithdrawLiq { + assets: pool.pool_assets(deps.as_ref())?, + receive_choice: receive_choice.clone(), + recipient: recipient.clone(), + } + .into_cosmos_msg(&env)?, + ), + vec![requested_asset.clone()], + )) + } else { + // Basket liquidate the asset withdrawn from the vault + let msgs = router.basket_liquidate_msgs( + vec![base_token].into(), + requested_asset, + None, // Not needed as we have our own min_out enforcement + Some(recipient.to_string()), + )?; + Ok(( + Response::new().add_messages(msgs), + vec![requested_asset.clone()], + )) + } } } - ReceiveChoice::BaseToken => { - Ok(Response::new().add_message(base_token.transfer_msg(recipient)?)) - } + ReceiveChoice::BaseToken => Ok(( + Response::new().add_message(base_token.transfer_msg(&recipient)?), + vec![base_token.info.clone()], + )), ReceiveChoice::Underlying => { if let Some(pool) = pool { let pool_assets = pool.pool_assets(deps.as_ref())?; let res = pool.withdraw_liquidity(deps.as_ref(), &env, base_token, AssetList::new())?; - Ok(res.add_message( - CallbackMsg::AfterWithdrawLiq { - assets: pool_assets, - receive_choice, - recipient, - min_out, - } - .into_cosmos_msg(&env)?, + Ok(( + res.add_message( + CallbackMsg::AfterWithdrawLiq { + assets: pool_assets.clone(), + receive_choice, + recipient: recipient.clone(), + } + .into_cosmos_msg(&env)?, + ), + pool_assets, )) } else { Err(ContractError::UnsupportedWithdrawal {}) } } + }?; + + // Add a message to enforce the minimum amount of assets received + let balances_before = + AssetList::query_asset_info_balances(withdrawal_assets.clone(), &deps.querier, &recipient)?; + let enforce_min_out_msg = CallbackMsg::EnforceMinOut { + assets: withdrawal_assets, + recipient: recipient.clone(), + balances_before, + min_out: min_out.clone(), } + .into_cosmos_msg(&env)?; + + Ok(res.add_message(enforce_min_out_msg)) } pub fn callback_after_withdraw_liq( @@ -211,7 +248,6 @@ pub fn callback_after_withdraw_liq( assets: Vec, receive_choice: ReceiveChoice, recipient: Addr, - min_out: AssetList, ) -> Result { let router = ROUTER.load(deps.storage)?; @@ -220,13 +256,9 @@ pub fn callback_after_withdraw_liq( match receive_choice { ReceiveChoice::SwapTo(requested_asset) => { - let min_out = unwrap_min_out(min_out, &requested_asset)?; - // Subtract the requested asset balance from min_out, as we will - // transfer this amount to the recipient. let requested_asset_balance = asset_balances .find(&requested_asset) .map_or(Uint128::zero(), |x| x.amount); - let min_out = min_out.saturating_sub(requested_asset_balance); // Add messages to basket liquidate the assets withdrawn from the LP, but filter // out the requested asset as we can't swap an asset to itself. @@ -238,7 +270,7 @@ pub fn callback_after_withdraw_liq( .collect::>() .into(), &requested_asset, - Some(min_out), + None, // Not needed as we have our own min_out enforcement Some(recipient.to_string()), )?; @@ -253,24 +285,6 @@ pub fn callback_after_withdraw_liq( Ok(Response::new().add_messages(msgs)) } ReceiveChoice::Underlying => { - // Verify min_out and then just send the assets to the recipient - for min_asset in min_out.into_iter() { - if asset_balances - .find(&min_asset.info) - .map(|x| x.amount) - .unwrap_or_default() - < min_asset.amount - { - return Err(ContractError::MinOutNotMet { - min_out: min_asset.amount, - actual: asset_balances - .find(&min_asset.info) - .map(|x| x.amount) - .unwrap_or_default(), - }); - } - } - let msgs = asset_balances.transfer_msgs(recipient)?; Ok(Response::new().add_messages(msgs)) } @@ -279,20 +293,3 @@ pub fn callback_after_withdraw_liq( } } } - -/// Unwraps a single asset amount from an AssetList. -fn unwrap_min_out( - min_out: AssetList, - requested_asset: &AssetInfo, -) -> Result { - // Since we are requesting a single asset out, make sure the min_out argument - // contains the requested asset. - if min_out.len() > 1 || (min_out.len() == 1 && &min_out.to_vec()[0].info != requested_asset) { - return Err(ContractError::InvalidMinOut {}); - } - if min_out.len() == 1 { - Ok(min_out.to_vec()[0].amount) - } else { - Ok(Uint128::zero()) - } -} diff --git a/tests/test_withdraw.rs b/tests/test_withdraw.rs index 59bcf72..9ec312a 100644 --- a/tests/test_withdraw.rs +++ b/tests/test_withdraw.rs @@ -1,4 +1,4 @@ -use apollo_cw_asset::{Asset, AssetInfo, AssetList}; +use apollo_cw_asset::{Asset, AssetInfo, AssetList, AssetUnchecked}; use common::setup; use cosmwasm_std::{Decimal, Uint128}; use cw_dex::traits::Pool; @@ -12,9 +12,11 @@ use vault_zapper::msg::ReceiveChoice; pub mod common; -#[test_case(0; "no lock")] -#[test_case(300; "with lockup")] -fn withdraw_base_token(lock_duration: u64) { +#[test_case(0, true; "no lock, via ReceiveChoice::SwapTo")] +#[test_case(0, false; "no lock, via ReceiveChoice::BaseToken")] +#[test_case(300, true; "with lockup, via ReceiveChoice::SwapTo")] +#[test_case(300, false; "with lockup, via ReceiveChoice::BaseToken")] +fn withdraw_base_token(lock_duration: u64, via_swap_to: bool) { let owned_runner: OwnedTestRunner = common::get_test_runner(); let runner = owned_runner.as_ref(); let (robot, admin) = setup(&runner, lock_duration); @@ -36,9 +38,33 @@ fn withdraw_base_token(lock_duration: u64) { .assert_vault_token_balance_gt(admin.address(), 0u128) .assert_base_token_balance_eq(admin.address(), balance - deposit_amount); - let receive_choice = ReceiveChoice::SwapTo(deposit_asset_info); + let receive_choice = if via_swap_to { + ReceiveChoice::SwapTo(deposit_asset_info.clone()) + } else { + ReceiveChoice::BaseToken + }; if lock_duration == 0 { - robot.zapper_redeem_all(None, receive_choice, AssetList::new(), Unwrap::Ok, &admin); + robot + .zapper_redeem_all( + None, + receive_choice.clone(), + vec![AssetUnchecked::new( + deposit_asset_info.clone().into(), + deposit_amount + Uint128::new(1), + )], + Unwrap::Err("Minimum amount not met"), + &admin, + ) + .zapper_redeem_all( + None, + receive_choice, + vec![AssetUnchecked::new( + deposit_asset_info.clone().into(), + deposit_amount, + )], + Unwrap::Ok, + &admin, + ); } else { robot .zapper_unlock_all(&admin) @@ -51,11 +77,25 @@ fn withdraw_base_token(lock_duration: u64) { &admin, ) .increase_time(lock_duration) + .zapper_withdraw_unlocked( + 0, + None, + receive_choice.clone(), + vec![AssetUnchecked::new( + deposit_asset_info.clone().into(), + deposit_amount + Uint128::new(1), + )], + Unwrap::Err("Minimum amount not met"), + &admin, + ) .zapper_withdraw_unlocked( 0, None, receive_choice, - AssetList::new(), + vec![AssetUnchecked::new( + deposit_asset_info.clone().into(), + deposit_amount, + )], Unwrap::Ok, &admin, ); @@ -98,7 +138,26 @@ fn withdraw_one_asset_in_pool(lock_duration: u64) { let receive_choice = ReceiveChoice::SwapTo(deposit_asset_info.clone()); if lock_duration == 0 { - robot.zapper_redeem_all(None, receive_choice, AssetList::new(), Unwrap::Ok, &admin); + robot.zapper_redeem_all( + None, + receive_choice.clone(), + vec![AssetUnchecked::new( + deposit_asset_info.clone().into(), + deposit_amount * (Decimal::one() - Decimal::permille(3)), + )], + Unwrap::Err("Minimum amount not met"), + &admin, + ); + robot.zapper_redeem_all( + None, + receive_choice, + vec![AssetUnchecked::new( + deposit_asset_info.clone().into(), + deposit_amount * (Decimal::one() - Decimal::permille(4)), + )], + Unwrap::Ok, + &admin, + ); } else { robot .zapper_unlock_all(&admin) @@ -111,11 +170,25 @@ fn withdraw_one_asset_in_pool(lock_duration: u64) { &admin, ) .increase_time(lock_duration) + .zapper_withdraw_unlocked( + 0, + None, + receive_choice.clone(), + vec![AssetUnchecked::new( + deposit_asset_info.clone().into(), + deposit_amount * (Decimal::one() - Decimal::permille(3)), + )], + Unwrap::Err("Minimum amount not met"), + &admin, + ) .zapper_withdraw_unlocked( 0, None, receive_choice, - AssetList::new(), + vec![AssetUnchecked::new( + deposit_asset_info.clone().into(), + deposit_amount * (Decimal::one() - Decimal::permille(4)), + )], Unwrap::Ok, &admin, ); @@ -243,7 +316,52 @@ fn redeem_both_assets_of_pool(lock_duration: u64) { let max_rel_diff = "0.000000001"; // One unit lost due to rounding if lock_duration == 0 { robot - .zapper_redeem_all(None, receive_choice, AssetList::new(), Unwrap::Ok, &admin) + .zapper_redeem_all( + None, + receive_choice.clone(), + vec![AssetUnchecked::new( + asset1.clone().into(), + asset1_deposit_amount, + )], + Unwrap::Err("Minimum amount not met"), + &admin, + ) + .zapper_redeem_all( + None, + receive_choice.clone(), + vec![AssetUnchecked::new( + asset2.clone().into(), + asset2_deposit_amount, + )], + Unwrap::Err("Minimum amount not met"), + &admin, + ) + .zapper_redeem_all( + None, + receive_choice.clone(), + vec![ + AssetUnchecked::new(asset1.clone().into(), asset1_deposit_amount), + AssetUnchecked::new(asset2.clone().into(), asset2_deposit_amount), + ], + Unwrap::Err("Minimum amount not met"), + &admin, + ) + .zapper_redeem_all( + None, + receive_choice, + vec![ + AssetUnchecked::new( + asset1.clone().into(), + asset1_deposit_amount - Uint128::one(), + ), + AssetUnchecked::new( + asset2.clone().into(), + asset2_deposit_amount - Uint128::one(), + ), + ], + Unwrap::Ok, + &admin, + ) .assert_vault_token_balance_eq(admin.address(), 0u128); } else { robot @@ -257,11 +375,53 @@ fn redeem_both_assets_of_pool(lock_duration: u64) { &admin, ) .increase_time(lock_duration) + .zapper_withdraw_unlocked( + 0, + None, + receive_choice.clone(), + vec![AssetUnchecked::new( + asset1.clone().into(), + asset1_deposit_amount, + )], + Unwrap::Err("Minimum amount not met"), + &admin, + ) + .zapper_withdraw_unlocked( + 0, + None, + receive_choice.clone(), + vec![AssetUnchecked::new( + asset2.clone().into(), + asset2_deposit_amount, + )], + Unwrap::Err("Minimum amount not met"), + &admin, + ) + .zapper_withdraw_unlocked( + 0, + None, + receive_choice.clone(), + vec![ + AssetUnchecked::new(asset1.clone().into(), asset1_deposit_amount), + AssetUnchecked::new(asset2.clone().into(), asset2_deposit_amount), + ], + Unwrap::Err("Minimum amount not met"), + &admin, + ) .zapper_withdraw_unlocked( 0, None, receive_choice, - AssetList::new(), + vec![ + AssetUnchecked::new( + asset1.clone().into(), + asset1_deposit_amount - Uint128::one(), + ), + AssetUnchecked::new( + asset2.clone().into(), + asset2_deposit_amount - Uint128::one(), + ), + ], Unwrap::Ok, &admin, )