From 5fdc426c487dd3b898fd607d7060b6d2b27dd1a5 Mon Sep 17 00:00:00 2001 From: bekauz Date: Fri, 14 Jul 2023 12:28:25 +0200 Subject: [PATCH 1/2] non optional withdrawer; migrate msg update --- contracts/clock/src/contract.rs | 2 +- contracts/covenant/src/contract.rs | 58 +++++------ contracts/covenant/src/suite_test/suite.rs | 2 +- contracts/depositor/src/contract.rs | 20 ++-- contracts/holder/src/contract.rs | 112 ++++++++++++--------- contracts/holder/src/msg.rs | 21 ++-- contracts/holder/src/state.rs | 2 +- contracts/holder/src/suite_tests/suite.rs | 11 +- contracts/holder/src/suite_tests/tests.rs | 16 ++- contracts/lper/src/contract.rs | 5 +- contracts/lper/src/msg.rs | 6 +- contracts/lper/src/suite_test/suite.rs | 35 ++++--- contracts/lper/src/suite_test/tests.rs | 51 +++++++--- contracts/ls/src/contract.rs | 40 ++------ 14 files changed, 207 insertions(+), 174 deletions(-) diff --git a/contracts/clock/src/contract.rs b/contracts/clock/src/contract.rs index 7f7b61f5..eb1e1dd6 100644 --- a/contracts/clock/src/contract.rs +++ b/contracts/clock/src/contract.rs @@ -157,7 +157,7 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result { + MigrateMsg::UpdateCodeId { data: _ } => { // This is a migrate message to update code id, // Data is optional base64 that we can parse to any data we would like in the future // let data: SomeStruct = from_binary(&data)?; diff --git a/contracts/covenant/src/contract.rs b/contracts/covenant/src/contract.rs index 4902cdd2..1ac66ea8 100644 --- a/contracts/covenant/src/contract.rs +++ b/contracts/covenant/src/contract.rs @@ -14,8 +14,8 @@ use crate::{ state::{ CLOCK_CODE, COVENANT_CLOCK_ADDR, COVENANT_DEPOSITOR_ADDR, COVENANT_HOLDER_ADDR, COVENANT_LP_ADDR, COVENANT_LS_ADDR, DEPOSITOR_CODE, HOLDER_CODE, LP_CODE, LS_CODE, - PRESET_CLOCK_FIELDS, PRESET_DEPOSITOR_FIELDS, PRESET_HOLDER_FIELDS, PRESET_LP_FIELDS, - PRESET_LS_FIELDS, POOL_ADDRESS, + POOL_ADDRESS, PRESET_CLOCK_FIELDS, PRESET_DEPOSITOR_FIELDS, PRESET_HOLDER_FIELDS, + PRESET_LP_FIELDS, PRESET_LS_FIELDS, }, }; @@ -51,7 +51,7 @@ pub fn instantiate( PRESET_HOLDER_FIELDS.save(deps.storage, &msg.preset_holder_fields)?; POOL_ADDRESS.save(deps.storage, &msg.pool_address)?; - + let clock_instantiate_tx = CosmosMsg::Wasm(WasmMsg::Instantiate { admin: Some(env.contract.address.to_string()), code_id: msg.preset_clock_fields.clock_code, @@ -63,11 +63,7 @@ pub fn instantiate( // instantiate clock first Ok(Response::default() .add_attribute("method", "instantiate") - .add_submessage(SubMsg::reply_always( - clock_instantiate_tx, - CLOCK_REPLY_ID, - )) - ) + .add_submessage(SubMsg::reply_always(clock_instantiate_tx, CLOCK_REPLY_ID))) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -99,18 +95,18 @@ pub fn handle_clock_reply(deps: DepsMut, env: Env, msg: Reply) -> Result Err(ContractError::ContractInstantiationError { contract: "clock".to_string(), @@ -125,15 +121,17 @@ pub fn handle_holder_reply(deps: DepsMut, env: Env, msg: Reply) -> Result { COVENANT_HOLDER_ADDR.save(deps.storage, &response.contract_address)?; - + let pool_address = POOL_ADDRESS.load(deps.storage)?; let code_id = LP_CODE.load(deps.storage)?; let clock_addr = COVENANT_CLOCK_ADDR.load(deps.storage)?; let preset_lp_fields = PRESET_LP_FIELDS.load(deps.storage)?; - let instantiate_msg = preset_lp_fields - .clone() - .to_instantiate_msg(clock_addr, response.contract_address, pool_address); + let instantiate_msg = preset_lp_fields.clone().to_instantiate_msg( + clock_addr, + response.contract_address, + pool_address, + ); let lp_instantiate_tx: CosmosMsg = CosmosMsg::Wasm(WasmMsg::Instantiate { admin: Some(env.contract.address.to_string()), @@ -167,10 +165,9 @@ pub fn handle_lp_reply(deps: DepsMut, env: Env, msg: Reply) -> Result Result Err(ContractError::ContractInstantiationError { contract: "lp".to_string(), @@ -224,8 +220,7 @@ pub fn handle_ls_reply(deps: DepsMut, env: Env, msg: Reply) -> Result Err(ContractError::ContractInstantiationError { contract: "ls".to_string(), @@ -233,8 +228,11 @@ pub fn handle_ls_reply(deps: DepsMut, env: Env, msg: Reply) -> Result Result { +pub fn handle_depositor_reply( + deps: DepsMut, + _env: Env, + msg: Reply, +) -> Result { deps.api.debug("WASMDEBUG: depositor reply"); let parsed_data = parse_reply_instantiate_data(msg); @@ -244,7 +242,9 @@ pub fn handle_depositor_reply(deps: DepsMut, env: Env, msg: Reply) -> Result Err(ContractError::ContractInstantiationError { contract: "depositor".to_string() }), + Err(_err) => Err(ContractError::ContractInstantiationError { + contract: "depositor".to_string(), + }), } } diff --git a/contracts/covenant/src/suite_test/suite.rs b/contracts/covenant/src/suite_test/suite.rs index 4ffd033d..67f7bd02 100644 --- a/contracts/covenant/src/suite_test/suite.rs +++ b/contracts/covenant/src/suite_test/suite.rs @@ -89,7 +89,7 @@ impl Default for SuiteBuilder { }, }, preset_holder_fields: covenant_holder::msg::PresetHolderFields { - withdrawer: Some(CREATOR_ADDR.to_string()), + withdrawer: CREATOR_ADDR.to_string(), holder_code: 1, label: "covenant_holder_contract".to_string(), }, diff --git a/contracts/depositor/src/contract.rs b/contracts/depositor/src/contract.rs index df398505..e31d0d9e 100644 --- a/contracts/depositor/src/contract.rs +++ b/contracts/depositor/src/contract.rs @@ -108,9 +108,7 @@ fn try_tick(deps: ExecuteDeps, env: Env, info: MessageInfo) -> NeutronResult try_register_gaia_ica(deps, env), ContractState::ICACreated => try_liquid_stake(deps, env, info, ica_address), - ContractState::LiquidStaked => { - try_receive_atom_from_ica(deps, env, info, ica_address) - } + ContractState::LiquidStaked => try_receive_atom_from_ica(deps, env, info, ica_address), ContractState::Complete => try_completed(deps), } } @@ -132,7 +130,7 @@ fn try_liquid_stake( }; STRIDE_ATOM_RECEIVER.update(deps.storage, |mut val| -> StdResult<_> { val.address = stride_ica_addr.clone(); - Ok(val) + Ok(val) })?; let fee = IbcFee { @@ -499,8 +497,7 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> StdResult LS_ADDRESS.save(deps.storage, &ls_address)?; } - Ok(Response::default() - .add_attribute("method", "update_config")) + Ok(Response::default().add_attribute("method", "update_config")) } MigrateMsg::UpdateCodeId { data: _ } => { // This is a migrate message to update code id, @@ -537,9 +534,7 @@ fn sudo_open_ack( )?; ICA_ADDRESS.save(deps.storage, &parsed_version.address)?; CONTRACT_STATE.save(deps.storage, &ContractState::ICACreated)?; - return Ok(Response::default() - .add_attribute("method", "sudo_open_ack") - ); + return Ok(Response::default().add_attribute("method", "sudo_open_ack")); } Err(StdError::generic_err("Can't parse counterparty_version")) } @@ -631,9 +626,7 @@ fn sudo_response(deps: DepsMut, request: RequestPacket, data: Binary) -> StdResu )?; } - Ok(Response::default() - .add_attribute("method", "sudo_response") - ) + Ok(Response::default().add_attribute("method", "sudo_response")) } fn sudo_timeout(deps: DepsMut, _env: Env, request: RequestPacket) -> StdResult { @@ -686,8 +679,7 @@ fn sudo_timeout(deps: DepsMut, _env: Env, request: RequestPacket) -> StdResult StdResult { diff --git a/contracts/holder/src/contract.rs b/contracts/holder/src/contract.rs index 76fe966d..b799e5f9 100644 --- a/contracts/holder/src/contract.rs +++ b/contracts/holder/src/contract.rs @@ -2,14 +2,15 @@ use astroport::pair::Cw20HookMsg; #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ - to_binary, BankMsg, Binary, Coin, Deps, DepsMut, Env, MessageInfo, Response, StdResult, WasmMsg, CosmosMsg, + to_binary, BankMsg, Binary, Coin, CosmosMsg, Deps, DepsMut, Env, MessageInfo, Response, + StdResult, WasmMsg, }; use cw2::set_contract_version; use cw20::{BalanceResponse, Cw20ExecuteMsg}; use crate::error::ContractError; use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg}; -use crate::state::{WITHDRAWER, LP_ADDRESS}; +use crate::state::{LP_ADDRESS, WITHDRAWER}; const CONTRACT_NAME: &str = "crates.io:covenant-holder"; const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -24,26 +25,23 @@ pub fn instantiate( set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; deps.api.debug("WASMDEBUG: holder instantiate"); - // We cannot deserialize the address without first validating it - let withdrawer = msg - .withdrawer - .map(|addr| deps.api.addr_validate(&addr)) - .transpose()?; - match withdrawer { - // If there is a withdrawer, save it to state - Some(addr) => WITHDRAWER.save(deps.storage, &addr)?, - // Error if no withdrawer - None => return Err(ContractError::NoInitialWithdrawer {}), - } - LP_ADDRESS.save(deps.storage, &msg.lp_address)?; + let withdrawer = deps.api.addr_validate(&msg.withdrawer)?; + let lp_addr = deps.api.addr_validate(&msg.lp_address)?; + + WITHDRAWER.save(deps.storage, &withdrawer)?; + LP_ADDRESS.save(deps.storage, &lp_addr)?; - Ok(Response::default().add_attribute("method", "instantiate")) + Ok(Response::default() + .add_attribute("method", "instantiate") + .add_attribute("withdrawer", withdrawer) + .add_attribute("lp_address", lp_addr)) } #[cfg_attr(not(feature = "library"), entry_point)] pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { QueryMsg::Withdrawer {} => Ok(to_binary(&WITHDRAWER.may_load(deps.storage)?)?), + QueryMsg::LpAddress {} => Ok(to_binary(&LP_ADDRESS.may_load(deps.storage)?)?), } } @@ -55,24 +53,34 @@ pub fn execute( msg: ExecuteMsg, ) -> Result { match msg { - ExecuteMsg::Withdraw { quantity } => withdraw(deps, env, info, quantity), + ExecuteMsg::WithdrawLiquidity {} => try_withdraw_liquidity(deps, env, info), + ExecuteMsg::Withdraw { quantity } => try_withdraw_balances(deps, env, info, quantity), } } -// /// should be sent to the LP token contract associated with the pool -// /// to withdraw liquidity from -fn try_withdraw_liquidity(deps: DepsMut, env: Env) -> Result { +/// should be sent to the LP token contract associated with the pool +/// to withdraw liquidity from +fn try_withdraw_liquidity( + deps: DepsMut, + env: Env, + info: MessageInfo, +) -> Result { deps.api.debug("WASMDEBUG: withdrawing liquidity"); + // we validate who is initiating the liquidity removal + let withdrawer = WITHDRAWER.load(deps.storage)?; + if withdrawer != info.sender { + return Err(ContractError::Unauthorized {}); + } + let lp_address = LP_ADDRESS.load(deps.storage)?; - let pair_info: astroport::asset::PairInfo = deps.querier.query_wasm_smart( - lp_address.to_string(), - &astroport::pair::QueryMsg::Pair {}, - )?; + let pair_info: astroport::asset::PairInfo = deps + .querier + .query_wasm_smart(lp_address.to_string(), &astroport::pair::QueryMsg::Pair {})?; let liquidity_token_balance: BalanceResponse = deps.querier.query_wasm_smart( - pair_info.liquidity_token.to_string(), + pair_info.clone().liquidity_token, &cw20::Cw20QueryMsg::Balance { address: env.contract.address.to_string(), }, @@ -80,34 +88,32 @@ fn try_withdraw_liquidity(deps: DepsMut, env: Env) -> Result>, ) -> Result { - let withdrawer = WITHDRAWER.load(deps.storage)?; - // Check if the sender is the withdrawer + let withdrawer = WITHDRAWER.load(deps.storage)?; if info.sender != withdrawer { return Err(ContractError::Unauthorized {}); } + // if quantity is specified let amount = if let Some(quantity) = quantity { quantity @@ -117,31 +123,43 @@ pub fn withdraw( // https://github.com/CosmWasm/wasmd/blob/master/x/wasm/internal/keeper/keeper.go#L185-L192 deps.querier.query_all_balances(env.contract.address)? }; + Ok(Response::new() + .add_attribute("method", "try_withdraw") .add_message(BankMsg::Send { to_address: withdrawer.to_string(), amount, - }) - .add_attribute("method", "withdraw")) + })) } #[cfg_attr(not(feature = "library"), entry_point)] -pub fn migrate(deps: DepsMut, env: Env, msg: MigrateMsg) -> Result { +pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> Result { deps.api.debug("WASMDEBUG: migrate"); match msg { - MigrateMsg::UpdateWithdrawer { withdrawer } => { - let withdrawer_addr = deps.api.addr_validate(&withdrawer)?; - WITHDRAWER.save(deps.storage, &withdrawer_addr)?; - - Ok(Response::default()) + MigrateMsg::UpdateConfig { + withdrawer, + lp_address, + } => { + let mut response = Response::default().add_attribute("method", "update_withdrawer"); + + if let Some(addr) = withdrawer { + WITHDRAWER.save(deps.storage, &deps.api.addr_validate(&addr)?)?; + response = response.add_attribute("withdrawer", addr); + } + + if let Some(addr) = lp_address { + LP_ADDRESS.save(deps.storage, &deps.api.addr_validate(&addr)?)?; + response = response.add_attribute("lp_address", addr); + } + + Ok(response) } MigrateMsg::UpdateCodeId { data: _ } => { // This is a migrate message to update code id, // Data is optional base64 that we can parse to any data we would like in the future // let data: SomeStruct = from_binary(&data)?; - Ok(Response::default()) + Ok(Response::default().add_attribute("method", "update_withdrawer")) } - MigrateMsg::WithdrawLiquidity { } => try_withdraw_liquidity(deps, env), } } diff --git a/contracts/holder/src/msg.rs b/contracts/holder/src/msg.rs index 7f49cbb9..409dfa7f 100644 --- a/contracts/holder/src/msg.rs +++ b/contracts/holder/src/msg.rs @@ -5,13 +5,13 @@ use cosmwasm_std::{Addr, Binary, Coin}; pub struct InstantiateMsg { /// A withdrawer is the only authorized address that can withdraw /// from the contract. Anyone can instantiate the contract. - pub withdrawer: Option, + pub withdrawer: String, pub lp_address: String, } #[cw_serde] pub struct PresetHolderFields { - pub withdrawer: Option, + pub withdrawer: String, pub holder_code: u64, pub label: String, } @@ -30,7 +30,10 @@ pub enum ExecuteMsg { /// The withdraw message can only be called by the withdrawer /// The withdraw can specify a quanity to be withdrawn. If no /// quantity is specified, the full balance is withdrawn - Withdraw { quantity: Option> }, + Withdraw { + quantity: Option>, + }, + WithdrawLiquidity {}, } #[cw_serde] @@ -39,11 +42,17 @@ pub enum QueryMsg { // Queries the withdrawer address #[returns(Addr)] Withdrawer {}, + #[returns(Addr)] + LpAddress {}, } #[cw_serde] pub enum MigrateMsg { - UpdateWithdrawer { withdrawer: String }, - UpdateCodeId { data: Option }, - WithdrawLiquidity {}, + UpdateConfig { + withdrawer: Option, + lp_address: Option, + }, + UpdateCodeId { + data: Option, + }, } diff --git a/contracts/holder/src/state.rs b/contracts/holder/src/state.rs index ac88da60..7685c139 100644 --- a/contracts/holder/src/state.rs +++ b/contracts/holder/src/state.rs @@ -2,4 +2,4 @@ use cosmwasm_std::Addr; use cw_storage_plus::Item; pub const WITHDRAWER: Item = Item::new("withdrawer"); -pub const LP_ADDRESS: Item = Item::new("lp_address"); \ No newline at end of file +pub const LP_ADDRESS: Item = Item::new("lp_address"); diff --git a/contracts/holder/src/suite_tests/suite.rs b/contracts/holder/src/suite_tests/suite.rs index 4f4ff915..b02c62b5 100644 --- a/contracts/holder/src/suite_tests/suite.rs +++ b/contracts/holder/src/suite_tests/suite.rs @@ -24,7 +24,7 @@ impl Default for SuiteBuilder { fn default() -> Self { Self { instantiate: InstantiateMsg { - withdrawer: Some(DEFAULT_WITHDRAWER.to_string()), + withdrawer: DEFAULT_WITHDRAWER.to_string(), lp_address: "stablepairpool".to_string(), }, app: App::default(), @@ -33,8 +33,13 @@ impl Default for SuiteBuilder { } impl SuiteBuilder { - pub fn with_withdrawer(mut self, w: Option) -> Self { - self.instantiate.withdrawer = w; + pub fn with_withdrawer(mut self, addr: String) -> Self { + self.instantiate.withdrawer = addr; + self + } + + pub fn with_lp(mut self, addr: String) -> Self { + self.instantiate.lp_address = addr; self } diff --git a/contracts/holder/src/suite_tests/tests.rs b/contracts/holder/src/suite_tests/tests.rs index 09fbebc4..53e946e6 100644 --- a/contracts/holder/src/suite_tests/tests.rs +++ b/contracts/holder/src/suite_tests/tests.rs @@ -12,9 +12,19 @@ fn test_instantiate_and_query_withdrawer() { } #[test] -#[should_panic(expected = "Initial withdrawer is required")] -fn test_instantiate_with_no_withdrawer() { - SuiteBuilder::default().with_withdrawer(None).build(); +#[should_panic(expected = "Invalid input: address not normalized")] +fn test_instantiate_invalid_withdrawer() { + SuiteBuilder::default() + .with_withdrawer("0Oo0Oo".to_string()) + .build(); +} + +#[test] +#[should_panic(expected = "Invalid input: address not normalized")] +fn test_instantiate_invalid_lp_addr() { + SuiteBuilder::default() + .with_lp("0Oo0Oo".to_string()) + .build(); } #[test] diff --git a/contracts/lper/src/contract.rs b/contracts/lper/src/contract.rs index a4177ce3..d8c29df8 100644 --- a/contracts/lper/src/contract.rs +++ b/contracts/lper/src/contract.rs @@ -383,7 +383,6 @@ fn try_get_single_side_lp_submsg( Ok(None) } - #[cfg_attr(not(feature = "library"), entry_point)] pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { @@ -417,9 +416,7 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> NeutronResult { // This is a migrate message to update code id, diff --git a/contracts/lper/src/msg.rs b/contracts/lper/src/msg.rs index 27560723..c9552c0a 100644 --- a/contracts/lper/src/msg.rs +++ b/contracts/lper/src/msg.rs @@ -1,6 +1,6 @@ use astroport::asset::{Asset, AssetInfo}; use cosmwasm_schema::{cw_serde, QueryResponses}; -use cosmwasm_std::{Addr, Decimal, Uint128, Binary}; +use cosmwasm_std::{Addr, Binary, Decimal, Uint128}; use covenant_clock_derive::clocked; use crate::state::ContractState; @@ -60,9 +60,7 @@ impl PresetLpFields { pool_address: String, ) -> InstantiateMsg { InstantiateMsg { - lp_position: LPInfo { - addr: pool_address, - }, + lp_position: LPInfo { addr: pool_address }, clock_address, holder_address, slippage_tolerance: self.slippage_tolerance, diff --git a/contracts/lper/src/suite_test/suite.rs b/contracts/lper/src/suite_test/suite.rs index 2fa144df..e009e0a0 100644 --- a/contracts/lper/src/suite_test/suite.rs +++ b/contracts/lper/src/suite_test/suite.rs @@ -99,7 +99,7 @@ fn holder_contract() -> Box> { covenant_holder::contract::instantiate, covenant_holder::contract::query, ) - .with_migrate(covenant_holder::contract::migrate) + .with_migrate(covenant_holder::contract::migrate), ) } @@ -225,7 +225,7 @@ impl Default for SuiteBuilder { tick_max_gas: Uint64::new(50000), }, holder_instantiate: covenant_holder::msg::InstantiateMsg { - withdrawer: Some(CREATOR_ADDR.to_string()), + withdrawer: CREATOR_ADDR.to_string(), // deterministic based on instantiate flow lp_address: "contract7".to_string(), }, @@ -297,7 +297,6 @@ impl SuiteBuilder { self.stablepair_instantiate.token_code_id = token_code; self.factory_instantiate.whitelist_code_id = whitelist_code; self.factory_instantiate.pair_configs[0].code_id = stablepair_code; - let whitelist_addr = app .instantiate_contract( @@ -551,14 +550,13 @@ impl Suite { } pub fn query_liquidity_token_addr(&self) -> astroport::asset::PairInfo { - self - .app - .wrap() - .query_wasm_smart( - self.stable_pair.1.to_string(), - &astroport::pair::QueryMsg::Pair {}, - ) - .unwrap() + self.app + .wrap() + .query_wasm_smart( + self.stable_pair.1.to_string(), + &astroport::pair::QueryMsg::Pair {}, + ) + .unwrap() } } @@ -676,12 +674,13 @@ impl Suite { } pub fn holder_withdraw(&mut self) { - self.app.migrate_contract( - Addr::unchecked(CREATOR_ADDR), - Addr::unchecked(self.holder_addr.to_string()), - &covenant_holder::msg::MigrateMsg::WithdrawLiquidity { }, - 8, - ) - .unwrap(); + self.app + .execute_contract( + Addr::unchecked(CREATOR_ADDR), + Addr::unchecked(self.holder_addr.to_string()), + &covenant_holder::msg::ExecuteMsg::WithdrawLiquidity {}, + &[], + ) + .unwrap(); } } diff --git a/contracts/lper/src/suite_test/tests.rs b/contracts/lper/src/suite_test/tests.rs index 9667eebf..ddc6dfda 100644 --- a/contracts/lper/src/suite_test/tests.rs +++ b/contracts/lper/src/suite_test/tests.rs @@ -36,7 +36,7 @@ fn test_instantiate_happy() { let liquidity_token_addr = pairinfo.liquidity_token.to_string(); - let holder_balances = suite.query_cw20_bal(liquidity_token_addr.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal(liquidity_token_addr, suite.holder_addr.to_string()); assert_eq!(Uint128::zero(), holder_balances.balance); suite.pass_blocks(10); @@ -57,14 +57,21 @@ fn test_instantiate_happy() { liquid_pooler_balances ); - let holder_balances = suite.query_cw20_bal(pairinfo.liquidity_token.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal( + pairinfo.liquidity_token.to_string(), + suite.holder_addr.to_string(), + ); assert_ne!(Uint128::zero(), holder_balances.balance); suite.holder_withdraw(); - let holder_balances = suite.query_cw20_bal(pairinfo.liquidity_token.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal( + pairinfo.liquidity_token.to_string(), + suite.holder_addr.to_string(), + ); assert_eq!(Uint128::zero(), holder_balances.balance); - let holder_native_balances = suite.query_addr_balances(Addr::unchecked(suite.holder_addr.to_string())); + let holder_native_balances = + suite.query_addr_balances(Addr::unchecked(suite.holder_addr.to_string())); assert_eq!(2, holder_native_balances.len()); assert_ne!(Uint128::zero(), holder_native_balances[0].amount); assert_ne!(Uint128::zero(), holder_native_balances[1].amount); @@ -102,7 +109,10 @@ fn test_exceeded_single_side_lp_ratio_first_asset_dominant() { ); let pairinfo = suite.query_liquidity_token_addr(); - let holder_balances = suite.query_cw20_bal(pairinfo.liquidity_token.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal( + pairinfo.liquidity_token.to_string(), + suite.holder_addr.to_string(), + ); assert_eq!(Uint128::zero(), holder_balances.balance); suite.tick(); @@ -152,14 +162,21 @@ fn test_exceeded_single_side_lp_ratio_first_asset_dominant() { .query_addr_balances(Addr::unchecked(suite.liquid_pooler.1.to_string())) .len() ); - let holder_balances = suite.query_cw20_bal(pairinfo.liquidity_token.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal( + pairinfo.liquidity_token.to_string(), + suite.holder_addr.to_string(), + ); assert_ne!(Uint128::zero(), holder_balances.balance); suite.holder_withdraw(); - let holder_balances = suite.query_cw20_bal(pairinfo.liquidity_token.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal( + pairinfo.liquidity_token.to_string(), + suite.holder_addr.to_string(), + ); assert_eq!(Uint128::zero(), holder_balances.balance); - let holder_native_balances = suite.query_addr_balances(Addr::unchecked(suite.holder_addr.to_string())); + let holder_native_balances = + suite.query_addr_balances(Addr::unchecked(suite.holder_addr.to_string())); assert_eq!(2, holder_native_balances.len()); assert_ne!(Uint128::zero(), holder_native_balances[0].amount); assert_ne!(Uint128::zero(), holder_native_balances[1].amount); @@ -186,7 +203,10 @@ fn test_exceeded_single_side_lp_ratio_second_asset_dominant() { ); let pairinfo = suite.query_liquidity_token_addr(); - let holder_balances = suite.query_cw20_bal(pairinfo.liquidity_token.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal( + pairinfo.liquidity_token.to_string(), + suite.holder_addr.to_string(), + ); assert_eq!(Uint128::zero(), holder_balances.balance); suite.tick(); @@ -216,14 +236,21 @@ fn test_exceeded_single_side_lp_ratio_second_asset_dominant() { .query_addr_balances(Addr::unchecked(suite.liquid_pooler.1.to_string())) .len() ); - let holder_balances = suite.query_cw20_bal(pairinfo.liquidity_token.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal( + pairinfo.liquidity_token.to_string(), + suite.holder_addr.to_string(), + ); assert_ne!(Uint128::zero(), holder_balances.balance); suite.holder_withdraw(); - let holder_balances = suite.query_cw20_bal(pairinfo.liquidity_token.to_string(), suite.holder_addr.to_string()); + let holder_balances = suite.query_cw20_bal( + pairinfo.liquidity_token.to_string(), + suite.holder_addr.to_string(), + ); assert_eq!(Uint128::zero(), holder_balances.balance); - let holder_native_balances = suite.query_addr_balances(Addr::unchecked(suite.holder_addr.to_string())); + let holder_native_balances = + suite.query_addr_balances(Addr::unchecked(suite.holder_addr.to_string())); assert_eq!(2, holder_native_balances.len()); assert_ne!(Uint128::zero(), holder_native_balances[0].amount); assert_ne!(Uint128::zero(), holder_native_balances[1].amount); diff --git a/contracts/ls/src/contract.rs b/contracts/ls/src/contract.rs index 8c7120c5..2611893e 100644 --- a/contracts/ls/src/contract.rs +++ b/contracts/ls/src/contract.rs @@ -2,7 +2,6 @@ use std::fmt::Error; use cosmos_sdk_proto::cosmos::base::v1beta1::Coin; use cosmos_sdk_proto::ibc::applications::transfer::v1::MsgTransfer; -use cosmos_sdk_proto::ibc::core::client::v1::Height; use cosmos_sdk_proto::traits::Message; #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; @@ -95,7 +94,7 @@ fn try_tick(deps: DepsMut, env: Env, info: MessageInfo) -> NeutronResult try_register_stride_ica(deps, env), @@ -184,22 +183,12 @@ fn try_execute_transfer( Ok(Response::default() .add_attribute("method", "try_execute_transfer") - .add_submessage(SubMsg::reply_on_success(submit_msg, TRANSFER_REPLY_ID)) - ) + .add_submessage(SubMsg::reply_on_success(submit_msg, TRANSFER_REPLY_ID))) } None => Err(NeutronError::Fmt(Error)), } } -fn try_completed(deps: DepsMut) -> NeutronResult> { - let clock_addr = CLOCK_ADDRESS.load(deps.storage)?; - let msg = covenant_clock::helpers::dequeue_msg(clock_addr.as_str())?; - - Ok(Response::default() - .add_attribute("method", "try_completed") - .add_message(msg)) -} - #[allow(unused)] fn msg_with_sudo_callback>, T>( deps: DepsMut, @@ -366,9 +355,7 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> StdResult LS_DENOM.save(deps.storage, &ls_denom)?; } - Ok(Response::default() - .add_attribute("method", "update_config") - ) + Ok(Response::default().add_attribute("method", "update_config")) } MigrateMsg::UpdateCodeId { data: _ } => { // This is a migrate message to update code id, @@ -405,9 +392,7 @@ fn sudo_open_ack( )?; ICA_ADDRESS.save(deps.storage, &parsed_version.address)?; CONTRACT_STATE.save(deps.storage, &ContractState::ICACreated)?; - return Ok(Response::default() - .add_attribute("method", "sudo_open_ack") - ); + return Ok(Response::default().add_attribute("method", "sudo_open_ack")); } Err(StdError::generic_err("Can't parse counterparty_version")) } @@ -452,8 +437,7 @@ fn sudo_response(deps: DepsMut, request: RequestPacket, data: Binary) -> StdResu add_error_to_queue(deps.storage, error_msg.to_string()); return Ok(Response::default() .add_attribute("method", "sudo_open_ack") - .add_attribute("error", "no_payload") - ); + .add_attribute("error", "no_payload")); } deps.api @@ -502,9 +486,7 @@ fn sudo_response(deps: DepsMut, request: RequestPacket, data: Binary) -> StdResu )?; } - Ok(Response::default() - .add_attribute("method", "sudo_response") - ) + Ok(Response::default().add_attribute("method", "sudo_response")) } fn sudo_timeout(deps: DepsMut, _env: Env, request: RequestPacket) -> StdResult { @@ -557,9 +539,7 @@ fn sudo_timeout(deps: DepsMut, _env: Env, request: RequestPacket) -> StdResult StdResult { @@ -605,9 +585,7 @@ fn sudo_error(deps: DepsMut, request: RequestPacket, details: String) -> StdResu add_error_to_queue(deps.storage, error_msg.to_string()); } - Ok(Response::default() - .add_attribute("method", "sudo_error") - ) + Ok(Response::default().add_attribute("method", "sudo_error")) } // prepare_sudo_payload is called from reply handler @@ -668,4 +646,4 @@ pub fn handle_transfer_reply(deps: DepsMut, _env: Env, msg: Reply) -> StdResult< } Ok(Response::default().add_attribute("method", "handle_transfer_reply")) -} \ No newline at end of file +} From 519315e6cc31fb1760825d9c033cd57181e4bceb Mon Sep 17 00:00:00 2001 From: bekauz Date: Fri, 14 Jul 2023 15:26:57 +0200 Subject: [PATCH 2/2] tests rework --- contracts/holder/src/contract.rs | 1 + contracts/holder/src/suite_tests/mod.rs | 2 +- contracts/holder/src/suite_tests/suite.rs | 29 ++-- contracts/holder/src/suite_tests/tests.rs | 167 +++++----------------- 4 files changed, 56 insertions(+), 143 deletions(-) diff --git a/contracts/holder/src/contract.rs b/contracts/holder/src/contract.rs index b799e5f9..eee42863 100644 --- a/contracts/holder/src/contract.rs +++ b/contracts/holder/src/contract.rs @@ -58,6 +58,7 @@ pub fn execute( } } +// this is tested in the LP module /// should be sent to the LP token contract associated with the pool /// to withdraw liquidity from fn try_withdraw_liquidity( diff --git a/contracts/holder/src/suite_tests/mod.rs b/contracts/holder/src/suite_tests/mod.rs index a46a3a90..89c24a7f 100644 --- a/contracts/holder/src/suite_tests/mod.rs +++ b/contracts/holder/src/suite_tests/mod.rs @@ -9,7 +9,7 @@ mod tests; // function where the assertion would otherwise happen. macro_rules! is_error { ($x:expr, $e:expr) => { - assert!(format!("{:#}", $x.unwrap_err()).contains($e)) + assert!(format!("{:#}", $x).contains($e)) }; } pub(crate) use is_error; diff --git a/contracts/holder/src/suite_tests/suite.rs b/contracts/holder/src/suite_tests/suite.rs index b02c62b5..f9836a8e 100644 --- a/contracts/holder/src/suite_tests/suite.rs +++ b/contracts/holder/src/suite_tests/suite.rs @@ -43,13 +43,6 @@ impl SuiteBuilder { self } - pub fn with_funded_user(mut self, user: Addr, amount: Vec) -> Self { - self.app = AppBuilder::new().build(|router, _, storage| { - router.bank.init_balance(storage, &user, amount).unwrap(); - }); - self - } - pub fn build(self) -> Suite { let mut app = self.app; let holder_code = app.store_code(holder_contract()); @@ -80,7 +73,7 @@ impl Suite { &mut self, caller: &str, quantity: Vec, - ) -> anyhow::Result { + ) -> AppResponse { self.app.execute_contract( Addr::unchecked(caller), self.holder.clone(), @@ -89,6 +82,7 @@ impl Suite { }, &[], ) + . unwrap() } /// sends a message on caller's behalf to withdraw remaining balance @@ -110,13 +104,26 @@ impl Suite { .query_wasm_smart(&self.holder, &QueryMsg::Withdrawer {}) .unwrap() } + + pub fn query_lp_address(&self) -> Addr { + self.app + .wrap() + .query_wasm_smart(&self.holder, &QueryMsg::LpAddress {}) + .unwrap() + } } // helper impl Suite { - pub fn fund_holder(&mut self, funder: Addr, tokens: Vec) -> anyhow::Result { - self.app.send_tokens(funder, self.holder.clone(), &tokens) - } + pub fn fund_holder(&mut self, tokens: Vec) -> AppResponse { + self.app.sudo(cw_multi_test::SudoMsg::Bank( + cw_multi_test::BankSudo::Mint { + to_address: self.holder.to_string(), + amount: tokens, + }, + )) + .unwrap() + } pub fn assert_holder_balance(&mut self, tokens: Vec) { for c in &tokens { diff --git a/contracts/holder/src/suite_tests/tests.rs b/contracts/holder/src/suite_tests/tests.rs index 53e946e6..b9acb0a0 100644 --- a/contracts/holder/src/suite_tests/tests.rs +++ b/contracts/holder/src/suite_tests/tests.rs @@ -1,6 +1,5 @@ -use super::is_error; use super::suite::{SuiteBuilder, DEFAULT_WITHDRAWER}; -use cosmwasm_std::{coin, coins, Addr, Coin}; +use cosmwasm_std::{coin, coins, Addr}; #[test] fn test_instantiate_and_query_withdrawer() { @@ -28,60 +27,23 @@ fn test_instantiate_invalid_lp_addr() { } #[test] -fn test_fund_contract_single_denom() { - // set up an initial user with a balance in the test suite - let user = Addr::unchecked("anyuser"); - let initial_user_balance = coins(1000, "coin"); +#[should_panic(expected = "Unauthorized")] +fn test_withdraw_all_unauthorized() { let mut suite = SuiteBuilder::default() - .with_funded_user(user.clone(), initial_user_balance) .build(); - // this user funds the holder contract - let amt_to_fund_contract = coins(100, "coin"); - suite - .fund_holder(user, amt_to_fund_contract.clone()) - .unwrap(); + suite.fund_holder(coins(100, "coin")); - // check that the holder contract balance has increased - suite.assert_holder_balance(amt_to_fund_contract); + // attacker attempts to withdraw, panic + suite.withdraw_all("attacker").unwrap(); } #[test] -fn test_fund_and_withdraw_all_unauthorized() { - // create an attacker with an unauthorized address - let unauthorized = Addr::unchecked("attacker"); - - // set up an initial user with a balance in the test suite - let user = Addr::unchecked("anyuser"); - let initial_user_balance = coins(1000, "coin"); +fn test_withdraw_all_single_denom() { let mut suite = SuiteBuilder::default() - .with_funded_user(user.clone(), initial_user_balance) .build(); - // this user funds the holder contract - let amt_to_fund_contract = coins(100, "coin"); - suite.fund_holder(user, amt_to_fund_contract).unwrap(); - - // attacker attempts to withdraw all - let resp = suite.withdraw_all(unauthorized.as_ref()); - is_error!(resp, "Unauthorized"); - - // check to see the balance is unchanged - suite.assert_holder_balance(coins(100, "coin")); -} - -#[test] -fn test_fund_and_withdraw_all_single_denom() { - // set up an initial user with a balance in the test suite - let user = Addr::unchecked("anyuser"); - let initial_user_balance = coins(1000, "coin"); - let mut suite = SuiteBuilder::default() - .with_funded_user(user.clone(), initial_user_balance) - .build(); - - // this user funds the holder contract - let amt_to_fund_contract = coins(100, "coin"); - suite.fund_holder(user, amt_to_fund_contract).unwrap(); + suite.fund_holder(coins(100, "coin")); // withdraw all suite.withdraw_all(DEFAULT_WITHDRAWER).unwrap(); @@ -94,135 +56,78 @@ fn test_fund_and_withdraw_all_single_denom() { } #[test] -fn test_fund_and_withdraw_all_two_denoms() { - // set up an initial user with a balance in the test suite - let user = Addr::unchecked("anyuser"); - let initial_user_balance: Vec = vec![coin(100, "atom"), coin(90, "statom")]; - +fn test_withdraw_all_two_denoms() { let mut suite = SuiteBuilder::default() - .with_funded_user(user.clone(), initial_user_balance) .build(); - // this user funds the holder contract - let amt_to_fund_contract: Vec = vec![coin(80, "atom"), coin(70, "statom")]; - - suite - .fund_holder(user, amt_to_fund_contract.clone()) - .unwrap(); + let balances = vec![coin(80, "atom"), coin(70, "statom")]; + suite.fund_holder(balances.clone()); // withdraw all suite.withdraw_all(DEFAULT_WITHDRAWER).unwrap(); - // check to see there is no balance - let expected_balance: Vec = vec![coin(0, "atom"), coin(0, "statom")]; - - suite.assert_holder_balance(expected_balance); - - // check to see holder has received everythning - suite.assert_withdrawer_balance(amt_to_fund_contract); + // assert all funds are now in withdrawer address + suite.assert_holder_balance(vec![coin(0, "atom"), coin(0, "statom")]); + suite.assert_withdrawer_balance(balances); } #[test] -fn test_fund_and_withdraw_partial_single_denom() { - // set up an initial user with a balance in the test suite - let user = Addr::unchecked("anyuser"); - let initial_user_balance = coins(1000, "coin"); +fn test_fund_single_withdraw_partial_single_denom() { let mut suite = SuiteBuilder::default() - .with_funded_user(user.clone(), initial_user_balance) .build(); - // this user funds the holder contract - let amt_to_fund_contract = coins(100, "coin"); - suite.fund_holder(user, amt_to_fund_contract).unwrap(); + suite.fund_holder(vec![coin(80, "atom")]); // withdraw 75 out of a total of 100 tokens suite - .withdraw_tokens(DEFAULT_WITHDRAWER, coins(75, "coin")) - .unwrap(); + .withdraw_tokens(DEFAULT_WITHDRAWER, coins(75, "atom")); // check to see there are 25 tokens left in contract - suite.assert_holder_balance(coins(25, "coin")); + suite.assert_holder_balance(coins(5, "atom")); // and holder has received 75 - suite.assert_withdrawer_balance(coins(75, "coin")); + suite.assert_withdrawer_balance(coins(75, "atom")); } #[test] -fn test_fund_and_withdraw_partial_two_denom() { - // set up an initial user with a balance in the test suite - let user = Addr::unchecked("anyuser"); - let initial_user_balance: Vec = vec![coin(100, "atom"), coin(90, "statom")]; - +fn test_fund_multi_denom_withdraw_partial_two_denom() { let mut suite = SuiteBuilder::default() - .with_funded_user(user.clone(), initial_user_balance) .build(); - // this user funds the holder contract - let amt_to_fund_contract: Vec = vec![coin(80, "atom"), coin(70, "statom")]; + let balances = vec![coin(80, "atom"), coin(70, "statom")]; + suite.fund_holder(balances); - suite.fund_holder(user, amt_to_fund_contract).unwrap(); + let amt_to_withdraw = vec![coin(50, "atom"), coin(30, "statom")]; - // withdraw partial - let amt_to_withdraw: Vec = vec![coin(50, "atom"), coin(30, "statom")]; - - suite - .withdraw_tokens(DEFAULT_WITHDRAWER, amt_to_withdraw.clone()) - .unwrap(); - - // check to see there is subtracted balance - let expected_balance: Vec = vec![coin(30, "atom"), coin(40, "statom")]; + suite.withdraw_tokens(DEFAULT_WITHDRAWER, amt_to_withdraw.clone()); + let expected_balance = vec![coin(30, "atom"), coin(40, "statom")]; suite.assert_holder_balance(expected_balance); - - // and that withdrawer has received withdrawn amount suite.assert_withdrawer_balance(amt_to_withdraw); } #[test] -fn test_fund_and_withdraw_exact_single_denom() { - // set up an initial user with a balance in the test suite - let user = Addr::unchecked("anyuser"); - let initial_user_balance = coins(1000, "coin"); +fn test_fund_multi_denom_withdraw_exact_single_denom() { let mut suite = SuiteBuilder::default() - .with_funded_user(user.clone(), initial_user_balance) .build(); - // this user funds the holder contract - let amt_to_fund_contract = coins(100, "coin"); - suite.fund_holder(user, amt_to_fund_contract).unwrap(); + let balances = vec![coin(80, "atom"), coin(70, "stuatom")]; + suite.fund_holder(balances); - // withdraw 100 out of a total of 100 tokens - suite - .withdraw_tokens(DEFAULT_WITHDRAWER, coins(100, "coin")) - .unwrap(); + suite.withdraw_tokens(DEFAULT_WITHDRAWER, coins(70, "stuatom")); // check to see there are 0 tokens left - suite.assert_holder_balance(coins(0, "coin")); + suite.assert_holder_balance(vec![coin(80, "atom")]); - // and withdrawer has them all - suite.assert_withdrawer_balance(coins(100, "coin")); + suite.assert_withdrawer_balance(coins(70, "stuatom")); } #[test] -fn test_fund_and_withdraw_too_big_single_denom() { - // set up an initial user with a balance in the test suite - let user = Addr::unchecked("anyuser"); - let initial_user_balance = coins(1000, "coin"); +#[should_panic(expected = "Cannot Sub with 70 and 100")] +fn test_fund_single_and_withdraw_too_big_single_denom() { let mut suite = SuiteBuilder::default() - .with_funded_user(user.clone(), initial_user_balance) .build(); + let holder_balances = vec![coin(80, "atom"), coin(70, "statom")]; + suite.fund_holder(holder_balances.clone()); - // this user funds the holder contract - let amt_to_fund_contract = coins(100, "coin"); - suite.fund_holder(user, amt_to_fund_contract).unwrap(); - - // try to withdraw 200 out of a total of 100 tokens - let resp = suite.withdraw_tokens(DEFAULT_WITHDRAWER, coins(200, "coin")); - // the dispatched bank send message should fail and everything should roll back - is_error!(resp, "error executing WasmMsg"); - - // check to see all tokens are intact - suite.assert_holder_balance(coins(100, "coin")); - - // and withdrawer has not received anything - suite.assert_withdrawer_balance(coins(0, "coin")); + suite.withdraw_tokens(DEFAULT_WITHDRAWER, coins(100, "statom")); }