From 3534411349e10b63d6115cbab85c91aad22637d1 Mon Sep 17 00:00:00 2001 From: Michael Pretorius Date: Fri, 30 Aug 2024 17:28:47 +0200 Subject: [PATCH 1/3] chore: add more events, remove native token validation --- ixo-swap/src/contract.rs | 243 ++++++++++++++++++++++++++----- ixo-swap/src/integration_test.rs | 104 ++++++++----- ixo-swap/src/msg.rs | 19 +++ 3 files changed, 293 insertions(+), 73 deletions(-) diff --git a/ixo-swap/src/contract.rs b/ixo-swap/src/contract.rs index 01ff612..90fa3c1 100644 --- a/ixo-swap/src/contract.rs +++ b/ixo-swap/src/contract.rs @@ -108,9 +108,22 @@ pub fn instantiate( let reply_msg = SubMsg::reply_on_success(instantiate_lp_token_msg, INSTANTIATE_LP_TOKEN_REPLY_ID); - Ok(Response::new().add_submessage(reply_msg)) + Ok(Response::new() + .add_submessage(reply_msg) + .add_attributes(vec![ + attr("action", "instantiate-ixo-swap"), + attr("owner", info.sender), + attr("max_slippage_percent", msg.max_slippage_percent.to_string()), + attr("lp_fee_percent", msg.lp_fee_percent.to_string()), + attr("protocol_fee_percent", msg.protocol_fee_percent.to_string()), + attr("protocol_fee_recipient", msg.protocol_fee_recipient), + attr("token_1155_denom", msg.token1155_denom.to_string()), + attr("token_2_denom", msg.token2_denom.to_string()), + ]) + ) } +/// Validates that slippage percent is not zero and less than max slippage percent fn validate_slippage_percent(percent: Decimal) -> Result<(), ContractError> { let max_slippage_percent = Decimal::from_str(PREDEFINED_MAX_SLIPPAGE_PERCENT)?; if percent.is_zero() || percent > max_slippage_percent { @@ -139,9 +152,14 @@ fn validate_input_tokens( }); } } - (Denom::Cw1155(token1155_addr, _), Denom::Native(native_denom)) => { + (Denom::Cw1155(token1155_addr, _), Denom::Native(_native_denom)) => { deps.api.addr_validate(token1155_addr.as_str())?; - validate_native_token_denom(&deps, native_denom)?; + // Removing this as we deem it unnecessary, since this validates against the bank modules + // registered DenomMetadata, but we want to allow any native denom to be used, including + // ibc tokens, without the need to add it to the bank modules DenomMetadata + // The audited security severity was minor since this just prevents against a misconfiguration + // by the contract instantiator, thus we deem it okay to bypass this validation + // validate_native_token_denom(&deps, native_denom)?; } _ => return Err(ContractError::InvalidTokenType {}), } @@ -279,22 +297,26 @@ fn execute_freeze_deposits( sender: Addr, freeze: bool, ) -> Result { + // validate that sender is owner if sender != OWNER.load(deps.storage)? { return Err(ContractError::UnauthorizedPoolFreeze {}); } + // update freeze status and save to storage if not same as current freeze status FROZEN.update(deps.storage, |freeze_status| -> Result<_, ContractError> { if freeze_status.eq(&freeze) { return Err(ContractError::DuplicatedFreezeStatus { freeze_status }); } Ok(freeze) })?; + Ok(Response::new().add_attributes(vec![ - attr("action", "freeze-contracts"), - attr("freeze_status", freeze.to_string()), + attr("action", "freeze-deposits"), + attr("frozen", freeze.to_string()), ])) } +/// Validates that expiration is not expired against block height or time fn check_expiration( expiration: &Option, block: &BlockInfo, @@ -310,6 +332,11 @@ fn check_expiration( } } +/// Calculates the amount of lp token to mint by: +/// +/// 1 - if liquidity supply is zero then return token1_amount +/// +/// 2 - if liquidity supply is not zero then return token1_amount * liquidity_supply / token1_reserve fn get_lp_token_amount_to_mint( token1_amount: Uint128, liquidity_supply: Uint128, @@ -326,6 +353,11 @@ fn get_lp_token_amount_to_mint( } } +/// Calculates the amount of token2 required for adding to liquidity pool by: +/// +/// 1 - if liquidity supply is zero then return max_token +/// +/// 2 - if liquidity supply is not zero then return token1_amount * token2_reserve / token1_reserve + 1 fn get_token2_amount_required( max_token: Uint128, token1_amount: Uint128, @@ -370,6 +402,7 @@ pub fn execute_add_liquidity( &info.sender, )?; + // calculate liquidity amount based on input amounts and do validation let token1155_total_amount = TokenAmount::Multiple(token1155_amounts.clone()).get_total(); let lp_token_supply = get_lp_token_supply(deps.as_ref(), &lp_token_addr)?; let liquidity_amount = @@ -385,6 +418,7 @@ pub fn execute_add_liquidity( token1155.reserve, )?; + // check that liquidity amount is more than users min liquidity if liquidity_amount < min_liquidity { return Err(ContractError::MinLiquidityError { min_liquidity, @@ -392,6 +426,7 @@ pub fn execute_add_liquidity( }); } + // check that token2 amount that will be used is less than max token provided by user if token2_amount > max_token2 { return Err(ContractError::MaxTokenError { max_token: max_token2, @@ -399,8 +434,8 @@ pub fn execute_add_liquidity( }); } - // Generate cw20 transfer messages if necessary let mut transfer_msgs: Vec = vec![]; + // add transfer message for 1155 tokens if let Denom::Cw1155(addr, _) = token1155.denom { transfer_msgs.push(get_cw1155_transfer_msg( &info.sender, @@ -409,6 +444,8 @@ pub fn execute_add_liquidity( &token1155_amounts, )?) } + + // if token2 is cw20 then add transfer message if let Denom::Cw20(addr) = token2.denom.clone() { transfer_msgs.push(get_cw20_transfer_from_msg( &info.sender, @@ -429,15 +466,17 @@ pub fn execute_add_liquidity( } } - TOKEN1155.update(deps.storage, |mut token| -> Result<_, ContractError> { + // update token reserves with newly added amounts + let updated_token1155 = TOKEN1155.update(deps.storage, |mut token| -> Result<_, ContractError> { token.reserve += token1155_total_amount; Ok(token) })?; - TOKEN2.update(deps.storage, |mut token| -> Result<_, ContractError> { + let updated_token2 = TOKEN2.update(deps.storage, |mut token| -> Result<_, ContractError> { token.reserve += token2_amount; Ok(token) })?; + // update lp token supplies to know what 1155 tokens is owned by the contract for (token_id, token_amount) in token1155_amounts.into_iter() { TOKEN_SUPPLIES.update( deps.storage, @@ -451,18 +490,24 @@ pub fn execute_add_liquidity( )?; } - let mint_msg = mint_lp_tokens(&info.sender, liquidity_amount, &lp_token_addr)?; + // mint lp tokens to user + transfer_msgs.push(mint_lp_tokens(&info.sender, liquidity_amount, &lp_token_addr)?); + Ok(Response::new() .add_messages(transfer_msgs) - .add_message(mint_msg) .add_attributes(vec![ attr("action", "add-liquidity"), attr("token1155_amount", token1155_total_amount), attr("token2_amount", token2_amount), attr("liquidity_received", liquidity_amount), + attr("liquidity_receiver", info.sender.to_string()), + attr("token1155_reserve", updated_token1155.reserve), + attr("token2_reserve", updated_token2.reserve), ])) } +/// Validates that native token denom is supported by the bank module's DebomMetadata +#[allow(dead_code)] fn validate_native_token_denom(deps: &DepsMut, denom: &String) -> Result<(), ContractError> { let denom_metadata: QueryDenomMetadataResponse = query_denom_metadata(deps.as_ref(), denom.clone())?; @@ -474,6 +519,8 @@ fn validate_native_token_denom(deps: &DepsMut, denom: &String) -> Result<(), Con Ok(()) } +/// Validates that 1155 tokens have supported denom as well as query each +/// token id from chain's Token module to ensure that it is a valid token fn validate_token1155_denom( deps: &DepsMut, denom: &Denom, @@ -498,6 +545,7 @@ fn validate_token1155_denom( Ok(()) } +/// Validates that min token is above zero fn validate_min_token(min_token: Uint128) -> Result<(), ContractError> { if min_token.is_zero() { return Err(ContractError::MinTokenError {}); @@ -506,6 +554,8 @@ fn validate_min_token(min_token: Uint128) -> Result<(), ContractError> { Ok(()) } +/// Validates that slippage is ok by calculating the minimum possible amount based on MAX_SLIPPAGE_PERCENT constant +/// and validates that min_token_amount is more than that minimum amount fn validate_slippage( deps: &DepsMut, min_token_amount: Uint128, @@ -529,6 +579,7 @@ fn validate_slippage( Ok(()) } +/// Creates a ixo1155 transfer message for all tokens in tokens hashmap fn get_cw1155_transfer_msg( owner: &Addr, recipient: &Addr, @@ -555,6 +606,7 @@ fn get_cw1155_transfer_msg( Ok(cw1155_transfer_cosmos_msg) } +/// Queries the total supply of lp token, which is cw20 contract on chain fn get_lp_token_supply(deps: Deps, lp_token_addr: &Addr) -> StdResult { let resp: cw20_lp::TokenInfoResponse = deps .querier @@ -562,6 +614,7 @@ fn get_lp_token_supply(deps: Deps, lp_token_addr: &Addr) -> StdResult { Ok(resp.total_supply) } +/// Creates a mint message for the given amount of lp token to the given recipient fn mint_lp_tokens( recipient: &Addr, liquidity_amount: Uint128, @@ -579,6 +632,7 @@ fn mint_lp_tokens( .into()) } +/// Queries the balance of the given cw20 contract on chain fn get_token_balance(deps: Deps, contract: &Addr, addr: &Addr) -> StdResult { let resp: cw20_lp::BalanceResponse = deps.querier.query_wasm_smart( contract, @@ -589,6 +643,7 @@ fn get_token_balance(deps: Deps, contract: &Addr, addr: &Addr) -> StdResult Result { - // create transfer cw20 msg let transfer_cw20_msg = Cw20ExecuteMsg::TransferFrom { owner: owner.into(), recipient: recipient.into(), @@ -635,6 +690,7 @@ fn get_cw20_transfer_from_msg( Ok(cw20_transfer_cosmos_msg) } +/// Creates a cw20 increase allowance message for the given token amount and spender fn get_cw20_increase_allowance_msg( token_addr: &Addr, spender: &Addr, @@ -660,12 +716,15 @@ pub fn execute_transfer_ownership( info: MessageInfo, new_owner: Option, ) -> Result { + // validate that sender is owner let owner = OWNER.load(deps.storage)?; if info.sender != owner { return Err(ContractError::Unauthorized {}); } let mut attributes = vec![attr("action", "transfer-ownership")]; + + // validate that new owner is valid and not same as current owner let new_owner_addr = new_owner .as_ref() .map(|h| deps.api.addr_validate(h)) @@ -675,9 +734,10 @@ pub fn execute_transfer_ownership( return Err(ContractError::DuplicatedOwner {}); } - attributes.push(attr("pending_owner", new_owner_addr)) + attributes.push(attr("pending_owner", new_owner_addr.to_string())) } + // save new owner to pending owner PENDING_OWNER.save(deps.storage, &new_owner_addr)?; Ok(Response::new().add_attributes(attributes)) @@ -690,14 +750,17 @@ pub fn execute_claim_ownership( let pending_owner = PENDING_OWNER.load(deps.storage)?; let mut attributes = vec![attr("action", "claim-ownership")]; + + // validate that sender is pending owner if let Some(pending_owner) = pending_owner { if info.sender != pending_owner { return Err(ContractError::Unauthorized {}); } + // save new owner to storage and remove pending owner PENDING_OWNER.save(deps.storage, &None)?; OWNER.save(deps.storage, &pending_owner)?; - attributes.push(attr("new_owner", pending_owner)); + attributes.push(attr("owner", pending_owner.to_string())); } Ok(Response::new().add_attributes(attributes)) @@ -708,11 +771,13 @@ pub fn execute_update_slippage( info: MessageInfo, max_slippage_percent: Decimal, ) -> Result { + // validate that sender is owner let owner = OWNER.load(deps.storage)?; if info.sender != owner { return Err(ContractError::Unauthorized {}); } + // validate slippage and save to storage validate_slippage_percent(max_slippage_percent)?; MAX_SLIPPAGE_PERCENT.save(deps.storage, &max_slippage_percent)?; @@ -729,11 +794,13 @@ pub fn execute_update_fee( protocol_fee_percent: Decimal, protocol_fee_recipient: String, ) -> Result { + // validate that sender is owner let owner = OWNER.load(deps.storage)?; if info.sender != owner { return Err(ContractError::Unauthorized {}); } + // validate that total fee percent is less than max fee percent let total_fee_percent = lp_fee_percent + protocol_fee_percent; let max_fee_percent = Decimal::from_str(PREDEFINED_MAX_PERCENT)?; if total_fee_percent > max_fee_percent { @@ -743,6 +810,7 @@ pub fn execute_update_fee( }); } + // update fees and save to storage let protocol_fee_recipient = deps.api.addr_validate(&protocol_fee_recipient)?; let updated_fees = Fees { protocol_fee_recipient: protocol_fee_recipient.clone(), @@ -752,7 +820,7 @@ pub fn execute_update_fee( FEES.save(deps.storage, &updated_fees)?; Ok(Response::new().add_attributes(vec![ - attr("action", "update-config"), + attr("action", "update-fee"), attr("lp_fee_percent", lp_fee_percent.to_string()), attr("protocol_fee_percent", protocol_fee_percent.to_string()), attr("protocol_fee_recipient", protocol_fee_recipient.to_string()), @@ -771,11 +839,13 @@ pub fn execute_remove_liquidity( check_expiration(&expiration, &env.block)?; let lp_token_addr = LP_ADDRESS.load(deps.storage)?; + // get users current liquidity tokens balance let balance = get_token_balance(deps.as_ref(), &lp_token_addr, &info.sender)?; let lp_token_supply = get_lp_token_supply(deps.as_ref(), &lp_token_addr)?; let token1155 = TOKEN1155.load(deps.storage)?; let token2 = TOKEN2.load(deps.storage)?; + // if amount user wants to remove is more than users balance, error if amount > balance { return Err(ContractError::InsufficientLiquidityError { requested: amount, @@ -787,11 +857,14 @@ pub fn execute_remove_liquidity( validate_min_token(min_token1155_total_amount)?; validate_min_token(min_token2)?; + // calculate 1155 amount user will get: amount * token1155_reserve / lp_token_supply let token1155_amount = amount .checked_mul(token1155.reserve) .map_err(StdError::overflow)? .checked_div(lp_token_supply) .map_err(StdError::divide_by_zero)?; + + // calculate token2 amount user will get: amount * token2_reserve / lp_token_supply let token2_amount = amount .checked_mul(token2.reserve) .map_err(StdError::overflow)? @@ -801,6 +874,7 @@ pub fn execute_remove_liquidity( validate_slippage(&deps, min_token1155_total_amount, token1155_amount)?; validate_slippage(&deps, min_token2, token2_amount)?; + // checks that output tokens is more than users minimum defined if token1155_amount < min_token1155_total_amount { return Err(ContractError::MinToken1155Error { requested: min_token1155_total_amount, @@ -814,15 +888,15 @@ pub fn execute_remove_liquidity( }); } - TOKEN1155.update(deps.storage, |mut token| -> Result<_, ContractError> { + // update token reserves by subtracting input amounts + let updated_token1155 = TOKEN1155.update(deps.storage, |mut token| -> Result<_, ContractError> { token.reserve = token .reserve .checked_sub(token1155_amount) .map_err(StdError::overflow)?; Ok(token) })?; - - TOKEN2.update(deps.storage, |mut token| -> Result<_, ContractError> { + let updated_token2 = TOKEN2.update(deps.storage, |mut token| -> Result<_, ContractError> { token.reserve = token .reserve .checked_sub(token2_amount) @@ -830,10 +904,12 @@ pub fn execute_remove_liquidity( Ok(token) })?; + // get the 1155 tokens to transfer, and update the TOKEN_SUPPLIES by subtracting all the tokens from the supply let token1155_amounts_to_transfer = get_token_amounts_to_transfer(deps.storage, token1155_amount, min_token1155)?; let mut msgs: Vec = vec![]; + // add transfer message for 1155 tokens if let Denom::Cw1155(addr, _) = token1155.denom { msgs.push(get_cw1155_transfer_msg( &env.contract.address, @@ -843,6 +919,7 @@ pub fn execute_remove_liquidity( )?) }; + // add transfer message for token2 match token2.denom { Denom::Cw20(addr) => msgs.push(get_cw20_transfer_to_msg( &info.sender, @@ -857,16 +934,26 @@ pub fn execute_remove_liquidity( _ => {} }; + // burn lp tokens from user msgs.push(get_burn_msg(&lp_token_addr, &info.sender, amount)?); Ok(Response::new().add_messages(msgs).add_attributes(vec![ attr("action", "remove-liquidity"), - attr("liquidity_burned", amount), attr("token1155_returned", token1155_amount), attr("token2_returned", token2_amount), + attr("liquidity_burned", amount), + attr("liquidity_provider", info.sender.to_string()), + attr("token1155_reserve", updated_token1155.reserve), + attr("token2_reserve", updated_token2.reserve), ])) } +/// Gets a map of 1155 tokens to transfer based on the token1155_amount to transfer by: +/// +/// 1 - if min_token1155 is single then it gets any random tokens from the TOKEN_SUPPLIES till amount is reached +/// +/// 2 - if min_token1155 is multiple then it gets the amount of tokens from the TOKEN_SUPPLIES that matches the multiple +/// inputs, otherwise it throws and error if the amount of tokens from the TOKEN_SUPPLIES is less than the amount fn get_token_amounts_to_transfer( storage: &mut dyn Storage, token1155_amount: Uint128, @@ -996,6 +1083,7 @@ fn update_token_amounts( update_token_supplies(storage, remaining_supply, token_id.clone(), token_supplies) } +/// Updates the TOKEN_SUPPLIES by subtracting the amount of tokens from the total supply and removing the token id if it is zero fn update_token_supplies( storage: &mut dyn Storage, remaining_supply: Uint128, @@ -1019,6 +1107,7 @@ fn update_token_supplies( Ok(()) } +/// Creates a burn message for the given amount of lp token from the given owner fn get_burn_msg(contract: &Addr, owner: &Addr, amount: Uint128) -> StdResult { let msg = cw20_base_lp::msg::ExecuteMsg::BurnFrom { owner: owner.to_string(), @@ -1032,6 +1121,7 @@ fn get_burn_msg(contract: &Addr, owner: &Addr, amount: Uint128) -> StdResult CosmosMsg { let transfer_bank_msg = cosmwasm_std::BankMsg::Send { to_address: recipient.into(), @@ -1064,6 +1155,7 @@ fn get_bank_transfer_to_msg(recipient: &Addr, denom: &str, native_amount: Uint12 transfer_bank_cosmos_msg } +/// Creates a transfer message for the given amount and denom, from sender to recipient fn get_fee_transfer_msg( sender: &Addr, recipient: &Addr, @@ -1085,6 +1177,17 @@ fn get_fee_transfer_msg( } } +/// Calculates the amount of tokens the user bought by:. +/// +/// 1 - if either reserve is zero throws error +/// +/// 2 - create fee percent with SCALE_FACTOR and calculate input_amount_with_fee +/// +/// 3 - calculate numerator: input_amount_with_fee * output_reserve +/// +/// 4 - calculate denominator: input_reserve * SCALE_FACTOR + input_amount_with_fee +/// +/// 5 - calculate amount bought: numerator / denominator fn get_input_price( input_amount: Uint128, input_reserve: Uint128, @@ -1113,6 +1216,13 @@ fn get_input_price( .try_into()?) } +/// Calculates the amount of tokens the reserves get, aka input amount minus the protocol fees, by: +/// +/// 1 - if fee amount is none then return input amount +/// +/// 2 - if fee amount is some then return input amount - fee amount +/// For single token input amount, it is input amount - fee amount +/// For multiple token input amount, it is input amount - fee amount for each token fn get_amount_without_fee( input_amount: &TokenAmount, fee_amount: Option, @@ -1154,6 +1264,7 @@ pub fn execute_swap( ) -> Result { check_expiration(&expiration, &env.block)?; + // map tokens to type and load from storage let input_token_item = match input_token_enum { TokenSelect::Token1155 => TOKEN1155, TokenSelect::Token2 => TOKEN2, @@ -1176,6 +1287,7 @@ pub fn execute_swap( let input_amount_total = input_amount.get_total(); let fees = FEES.load(deps.storage)?; let total_fee_percent = fees.lp_fee_percent + fees.protocol_fee_percent; + // get the calculated amount of tokens bought let mut token_bought = get_input_price( input_amount_total, input_token.reserve, @@ -1185,18 +1297,21 @@ pub fn execute_swap( validate_slippage(&deps, min_token_total, token_bought)?; + // check that token_bought is more than min_token_total provided by user if min_token_total > token_bought { return Err(ContractError::SwapMinError { min: min_token_total, available: token_bought, }); } - // Calculate fees + let protocol_fee_amount = input_amount.get_percent(fees.protocol_fee_percent)?; let input_amount_without_protocol_fee = get_amount_without_fee(&input_amount, protocol_fee_amount.clone())?; let mut msgs = vec![]; + // switch on input token denom and add transfer message from user to contract + // no need for native transfer as info.funds is same as input amount and will be transfered to contract match input_token.denom.clone() { Denom::Cw1155(addr, _) => msgs.push(get_cw1155_transfer_msg( &info.sender, @@ -1224,7 +1339,7 @@ pub fn execute_swap( } let recipient = deps.api.addr_validate(&recipient)?; - // Create transfer to message + // switch on output token denom and add transfer message from contract to recipient(user) msgs.push(match output_token.denom { Denom::Cw1155(addr, _) => { let tokens_to_transfer = @@ -1242,7 +1357,8 @@ pub fn execute_swap( Denom::Native(denom) => get_bank_transfer_to_msg(&recipient, &denom, token_bought), }); - input_token_item.update( + // update input token reserve adding input amount without protocol fee + let updated_input_token = input_token_item.update( deps.storage, |mut input_token| -> Result<_, ContractError> { let input_amount_without_protocol_fee_total = @@ -1256,7 +1372,8 @@ pub fn execute_swap( }, )?; - output_token_item.update( + // update output token reserve by subtracting token_bought + let updated_output_token = output_token_item.update( deps.storage, |mut output_token| -> Result<_, ContractError> { output_token.reserve = output_token @@ -1267,6 +1384,7 @@ pub fn execute_swap( }, )?; + // update lp token supplies by adding input amount if it multiple as it is 1155 tokens then and need to keep track of id and amount if let TokenAmount::Multiple(input_amounts) = input_amount_without_protocol_fee { for (token_id, token_amount) in input_amounts.into_iter() { TOKEN_SUPPLIES.update( @@ -1282,12 +1400,30 @@ pub fn execute_swap( } } - Ok(Response::new().add_messages(msgs).add_attributes(vec![ + // Attributes for response + let mut attributes = vec![ attr("action", "swap"), - attr("recipient", recipient), - attr("token_sold", input_amount_total), - attr("token_bought", token_bought), - ])) + attr("sender", info.sender.to_string()), + attr("recipient", recipient.to_string()), + attr("input_token_enum", input_token_enum.to_string()), + attr("input_token_amount", input_amount_total), + attr("output_token_amount", token_bought), + ]; + + // Add updated reserves based on the token type + match input_token_enum { + TokenSelect::Token1155 => { + attributes.push(attr("token1155_reserve", updated_input_token.reserve)); + attributes.push(attr("token2_reserve", updated_output_token.reserve)); + } + TokenSelect::Token2 => { + attributes.push(attr("token1155_reserve", updated_output_token.reserve)); + attributes.push(attr("token2_reserve", updated_input_token.reserve)); + } + } + + + Ok(Response::new().add_messages(msgs).add_attributes(attributes)) } #[allow(clippy::too_many_arguments)] @@ -1303,6 +1439,7 @@ pub fn execute_pass_through_swap( ) -> Result { check_expiration(&expiration, &env.block)?; + // map tokens to type and load from storage let input_token_state = match input_token_enum { TokenSelect::Token1155 => TOKEN1155, TokenSelect::Token2 => TOKEN2, @@ -1332,6 +1469,7 @@ pub fn execute_pass_through_swap( let input_token_amount_total = input_token_amount.get_total(); let fees = FEES.load(deps.storage)?; let total_fee_percent = fees.lp_fee_percent + fees.protocol_fee_percent; + // get the calculated amount of tokens bought let amount_to_transfer = get_input_price( input_token_amount_total, input_token.reserve, @@ -1374,7 +1512,7 @@ pub fn execute_pass_through_swap( let output_amm_address = deps.api.addr_validate(&output_amm_address)?; - // Increase allowance of output contract is transfer token is cw20 + // Increase allowance of output contract if transfer token is cw20 if let Denom::Cw20(addr) = &transfer_token.denom { msgs.push(get_cw20_increase_allowance_msg( addr, @@ -1384,6 +1522,7 @@ pub fn execute_pass_through_swap( )?) }; + // query info of output amm let resp: InfoResponse = deps .querier .query_wasm_smart(&output_amm_address, &QueryMsg::Info {})?; @@ -1396,6 +1535,7 @@ pub fn execute_pass_through_swap( Err(ContractError::InvalidOutputPool {}) }?; + // create swap and send to message let swap_msg = ExecuteMsg::SwapAndSendTo { input_token: transfer_input_token_enum, input_amount: TokenAmount::Single(amount_to_transfer), @@ -1404,9 +1544,10 @@ pub fn execute_pass_through_swap( expiration, }; + let output_amm_address_clone = output_amm_address.clone(); msgs.push( WasmMsg::Execute { - contract_addr: output_amm_address.into(), + contract_addr: output_amm_address_clone.into(), msg: to_json_binary(&swap_msg)?, funds: match transfer_token.denom { Denom::Native(denom) => vec![Coin { @@ -1419,7 +1560,8 @@ pub fn execute_pass_through_swap( .into(), ); - input_token_state.update(deps.storage, |mut token| -> Result<_, ContractError> { + // update input token reserve by adding input amount without protocol fee + let updated_input_token = input_token_state.update(deps.storage, |mut token| -> Result<_, ContractError> { // Add input amount - protocol fee to input token reserve let input_amount_without_protocol_fee_total = input_amount_without_protocol_fee.get_total(); token.reserve = token @@ -1430,7 +1572,8 @@ pub fn execute_pass_through_swap( Ok(token) })?; - transfer_token_state.update(deps.storage, |mut token| -> Result<_, ContractError> { + // update output token reserve by subtracting amount to transfer + let updated_transfer_token = transfer_token_state.update(deps.storage, |mut token| -> Result<_, ContractError> { token.reserve = token .reserve .checked_sub(amount_to_transfer) @@ -1439,6 +1582,7 @@ pub fn execute_pass_through_swap( Ok(token) })?; + // update lp token supplies by adding input amount if it multiple as it is 1155 tokens then and need to keep track of id and amount if let TokenAmount::Multiple(input_amounts) = input_amount_without_protocol_fee { for (token_id, token_amount) in input_amounts.into_iter() { TOKEN_SUPPLIES.update( @@ -1454,11 +1598,29 @@ pub fn execute_pass_through_swap( } } - Ok(Response::new().add_messages(msgs).add_attributes(vec![ + // Attributes for response + let mut attributes = vec![ attr("action", "cross-contract-swap"), + attr("input_token_enum", input_token_enum.to_string()), attr("input_token_amount", input_token_amount_total), - attr("native_transferred", amount_to_transfer), - ])) + attr("output_token_amount", amount_to_transfer), + attr("output_amm_address", output_amm_address.to_string()), + attr("recipient", info.sender.to_string()), + ]; + + // Add updated reserves based on the token type + match input_token_enum { + TokenSelect::Token1155 => { + attributes.push(attr("token1155_reserve", updated_input_token.reserve)); + attributes.push(attr("token2_reserve", updated_transfer_token.reserve)); + } + TokenSelect::Token2 => { + attributes.push(attr("token1155_reserve", updated_transfer_token.reserve)); + attributes.push(attr("token2_reserve", updated_input_token.reserve)); + } + } + + Ok(Response::new().add_messages(msgs).add_attributes(attributes)) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -1495,6 +1657,7 @@ pub fn query_denom_metadata(deps: Deps, denom: String) -> StdResult, @@ -1510,6 +1673,7 @@ pub fn query_tokens_supply( Ok(TokenSuppliesResponse { supplies }) } +/// Queries the MAX slippage percent pub fn query_slippage(deps: Deps) -> StdResult { let max_slippage_percent = MAX_SLIPPAGE_PERCENT.load(deps.storage)?; @@ -1518,6 +1682,7 @@ pub fn query_slippage(deps: Deps) -> StdResult { }) } +/// Queries the info of the contract, includes token reserves/denoms and lp supply/token address pub fn query_info(deps: Deps) -> StdResult { let token1155 = TOKEN1155.load(deps.storage)?; let token2 = TOKEN2.load(deps.storage)?; @@ -1533,6 +1698,7 @@ pub fn query_info(deps: Deps) -> StdResult { }) } +/// Queries the ownership of the contract, includes owner and pending owner pub fn query_ownership(deps: Deps) -> StdResult { let owner = OWNER.load(deps.storage)?.to_string(); let pending_owner = PENDING_OWNER.load(deps.storage)?.map(|o| o.into_string()); @@ -1543,6 +1709,7 @@ pub fn query_ownership(deps: Deps) -> StdResult { }) } +/// Queries the freeze status of the contract pub fn query_freeze_status(deps: Deps) -> StdResult { let freeze_status = FROZEN.load(deps.storage)?; @@ -1551,6 +1718,7 @@ pub fn query_freeze_status(deps: Deps) -> StdResult { }) } +/// Queries the price in token2 for the wanted token1155 amount pub fn query_token1155_for_token2_price( deps: Deps, token1155_amount: TokenAmount, @@ -1571,6 +1739,7 @@ pub fn query_token1155_for_token2_price( Ok(Token1155ForToken2PriceResponse { token2_amount }) } +/// Queries the price in token1155 for the wanted token2 amount pub fn query_token2_for_token1155_price( deps: Deps, token2_amount: TokenAmount, @@ -1591,6 +1760,7 @@ pub fn query_token2_for_token1155_price( Ok(Token2ForToken1155PriceResponse { token1155_amount }) } +/// Queries the fees of the contract, includes lp fee percent, protocol fee percent and protocol fee recipient pub fn query_fee(deps: Deps) -> StdResult { let fees = FEES.load(deps.storage)?; @@ -1615,7 +1785,10 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result Err(ContractError::InstantiateLpTokenError {}), } diff --git a/ixo-swap/src/integration_test.rs b/ixo-swap/src/integration_test.rs index 96b640f..dfcec78 100644 --- a/ixo-swap/src/integration_test.rs +++ b/ixo-swap/src/integration_test.rs @@ -172,14 +172,15 @@ fn create_amm( // set up amm contract let cw20_id = router.store_code(contract_cw20()); let amm_id = router.store_code(contract_amm()); + let msg = InstantiateMsg { - token1155_denom, - token2_denom, + token1155_denom: token1155_denom.clone(), + token2_denom: token2_denom.clone(), lp_token_code_id: cw20_id, max_slippage_percent, lp_fee_percent, protocol_fee_percent, - protocol_fee_recipient, + protocol_fee_recipient: protocol_fee_recipient.clone(), }; let init_msg = to_json_binary(&msg).unwrap(); let msg = WasmMsg::Instantiate { @@ -190,11 +191,22 @@ fn create_amm( label: "amm".to_string(), }; let res = router.execute(owner.clone(), msg.into()).unwrap(); - let event = Event::new("wasm").add_attribute( - "liquidity_pool_token_address", - format!("contract{}", cw20_id), - ); - res.has_event(&event); + let event = Event::new("wasm").add_attributes(vec![ + attr("action", "instantiate-ixo-swap"), + attr("owner", owner.to_string()), + attr("max_slippage_percent", max_slippage_percent.to_string()), + attr("lp_fee_percent", lp_fee_percent.to_string()), + attr("protocol_fee_percent", protocol_fee_percent.to_string()), + attr("protocol_fee_recipient", protocol_fee_recipient), + attr("token_1155_denom", token1155_denom.to_string()), + attr("token_2_denom", token2_denom.to_string()), + ]); + assert!(res.has_event(&event)); + let event = Event::new("wasm").add_attributes(vec![ + attr("action", "instantiate-lp-token"), + attr("liquidity_pool_token_address", format!("contract{}", cw20_id)), + ]); + assert!(res.has_event(&event)); let data = parse_instantiate_response_data(res.data.unwrap_or_default().as_slice()).unwrap(); Addr::unchecked(data.contract_address) @@ -314,27 +326,28 @@ fn instantiate() { assert_eq!(fee.protocol_fee_percent, protocol_fee_percent); assert_eq!(fee.protocol_fee_recipient, owner.to_string()); - // try instantiate with unsupported native denom - let cw20_id = router.store_code(contract_cw20()); - let amm_id = router.store_code(contract_amm()); - let msg = InstantiateMsg { - token1155_denom: Denom::Cw1155(cw1155_token.clone(), supported_denom.clone()), - token2_denom: Denom::Native("Unsupported".to_string()), - lp_token_code_id: cw20_id, - max_slippage_percent, - lp_fee_percent, - protocol_fee_percent, - protocol_fee_recipient: owner.to_string(), - }; - let err = router - .instantiate_contract(amm_id, owner.clone(), &msg, &[], "amm", None) - .unwrap_err(); - assert_eq!( - ContractError::UnsupportedTokenDenom { - id: "Unsupported".to_string() - }, - err.downcast().unwrap() - ); + // commenting this test case as removed bank DenomMetadata validation for native denoms + // // try instantiate with unsupported native denom + // let cw20_id = router.store_code(contract_cw20()); + // let amm_id = router.store_code(contract_amm()); + // let msg = InstantiateMsg { + // token1155_denom: Denom::Cw1155(cw1155_token.clone(), supported_denom.clone()), + // token2_denom: Denom::Native("Unsupported".to_string()), + // lp_token_code_id: cw20_id, + // max_slippage_percent, + // lp_fee_percent, + // protocol_fee_percent, + // protocol_fee_recipient: owner.to_string(), + // }; + // let err = router + // .instantiate_contract(amm_id, owner.clone(), &msg, &[], "amm", None) + // .unwrap_err(); + // assert_eq!( + // ContractError::UnsupportedTokenDenom { + // id: "Unsupported".to_string() + // }, + // err.downcast().unwrap() + // ); // try instantiate with duplicated tokens let cw20_id = router.store_code(contract_cw20()); @@ -660,8 +673,13 @@ fn cw1155_to_cw1155_swap() { .unwrap(); let event = Event::new("wasm").add_attributes(vec![ attr("action", "cross-contract-swap"), + attr("input_token_enum", "Token1155"), attr("input_token_amount", Uint128::new(50)), - attr("native_transferred", Uint128::new(33)), + attr("output_token_amount", Uint128::new(33)), + attr("output_amm_address", amm2.to_string()), + attr("recipient", owner.to_string()), + attr("token1155_reserve", Uint128::new(150)), // prev amount(100) plus added amount(50) + attr("token2_reserve", Uint128::new(67)), // prev amount(100) minus removed amount(33) ]); assert!(res.has_event(&event)); @@ -858,9 +876,13 @@ fn cw1155_to_cw20_swap() { .unwrap(); let event = Event::new("wasm").add_attributes(vec![ attr("action", "swap"), + attr("sender", owner.clone()), attr("recipient", owner.clone()), - attr("token_sold", Uint128::new(50_000)), - attr("token_bought", Uint128::new(33_266)), + attr("input_token_enum", "Token1155"), + attr("input_token_amount", Uint128::new(50_000)), + attr("output_token_amount", Uint128::new(33_266)), + attr("token1155_reserve", Uint128::new(149950)), // prev amount(100_000) plus added amount(50_000) - minus fees(50) + attr("token2_reserve", Uint128::new(66_734)), // prev amount(100_000) minus removed amount(33_266) ]); assert!(res.has_event(&event)); @@ -1284,6 +1306,9 @@ fn amm_add_and_remove_liquidity() { attr("token1155_amount", Uint128::new(100)), attr("token2_amount", Uint128::new(100)), attr("liquidity_received", Uint128::new(100)), + attr("liquidity_receiver", owner.to_string()), + attr("token1155_reserve", Uint128::new(100)), + attr("token2_reserve", Uint128::new(100)), ]); assert!(res.has_event(&event)); @@ -1484,11 +1509,14 @@ fn amm_add_and_remove_liquidity() { let res = router .execute_contract(owner.clone(), amm_addr.clone(), &remove_liquidity_msg, &[]) .unwrap(); - let event = Event::new("wasm").add_attributes(vec![ + let event: Event = Event::new("wasm").add_attributes(vec![ attr("action", "remove-liquidity"), - attr("liquidity_burned", Uint128::new(50)), attr("token1155_returned", Uint128::new(50)), attr("token2_returned", Uint128::new(50)), + attr("liquidity_burned", Uint128::new(50)), + attr("liquidity_provider", owner.to_string()), + attr("token1155_reserve", Uint128::new(100)), // prev amount(150) minus removed amount(50) + attr("token2_reserve", Uint128::new(101)), // prev amount(151) minus removed amount(50) ]); assert!(res.has_event(&event)); @@ -1837,8 +1865,8 @@ fn freeze_pool() { .execute_contract(owner.clone(), amm_addr.clone(), &freeze_msg, &[]) .unwrap(); let event = Event::new("wasm").add_attributes(vec![ - attr("action", "freeze-contracts"), - attr("freeze_status", "true"), + attr("action", "freeze-deposits"), + attr("frozen", "true"), ]); assert!(res.has_event(&event)); @@ -1961,7 +1989,7 @@ fn transfer_ownership() { .unwrap(); let event = Event::new("wasm").add_attributes(vec![ attr("action", "claim-ownership"), - attr("new_owner", new_owner.to_string()), + attr("owner", new_owner.to_string()), ]); assert!(res.has_event(&event)); @@ -2103,7 +2131,7 @@ fn update_fee() { .execute_contract(owner.clone(), amm_addr.clone(), &msg, &[]) .unwrap(); let event = Event::new("wasm").add_attributes(vec![ - attr("action", "update-config"), + attr("action", "update-fee"), attr("lp_fee_percent", lp_fee_percent.to_string()), attr("protocol_fee_percent", protocol_fee_percent.to_string()), attr("protocol_fee_recipient", "new_fee_recipient".to_string()), diff --git a/ixo-swap/src/msg.rs b/ixo-swap/src/msg.rs index 4f1de1d..aeb0ef2 100644 --- a/ixo-swap/src/msg.rs +++ b/ixo-swap/src/msg.rs @@ -216,3 +216,22 @@ pub struct QueryDenomMetadataResponse { #[prost(message, optional, tag = "1")] pub metadata: ::core::option::Option, } + +impl Denom { + pub fn to_string(&self) -> String { + match self { + Denom::Native(native_denom) => format!("Native:{}", native_denom), + Denom::Cw20(addr) => format!("Cw20:{}", addr), + Denom::Cw1155(addr, id) => format!("Cw1155:{}:{}", addr, id), + } + } +} + +impl TokenSelect { + pub fn to_string(&self) -> String { + match self { + TokenSelect::Token1155 => "Token1155".to_string(), + TokenSelect::Token2 => "Token2".to_string(), + } + } +} \ No newline at end of file From 45f974ec0a1e13faf5a0935c6e31651ae6631a0a Mon Sep 17 00:00:00 2001 From: Michael Pretorius Date: Wed, 4 Sep 2024 18:59:04 +0200 Subject: [PATCH 2/3] fix: fixing 1155 token selection, fee calculations, non-deterministic 1155 token selection --- ixo-swap/src/contract.rs | 393 +++++++++++++++++++++---------- ixo-swap/src/error.rs | 18 ++ ixo-swap/src/integration_test.rs | 266 +++++++++++++++++++-- ixo-swap/src/msg.rs | 4 +- ixo-swap/src/token_amount.rs | 113 +++++---- ixo-swap/src/utils.rs | 7 +- 6 files changed, 613 insertions(+), 188 deletions(-) diff --git a/ixo-swap/src/contract.rs b/ixo-swap/src/contract.rs index 90fa3c1..2135d07 100644 --- a/ixo-swap/src/contract.rs +++ b/ixo-swap/src/contract.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::convert::TryInto; use std::str::FromStr; @@ -27,7 +27,7 @@ use crate::state::{ }; use crate::token_amount::TokenAmount; use crate::utils::{ - decimal_to_uint128, PREDEFINED_MAX_PERCENT, PREDEFINED_MAX_SLIPPAGE_PERCENT, SCALE_FACTOR, + decimal_to_uint128, MIN_FEE_PERCENT, PREDEFINED_MAX_FEES_PERCENT, PREDEFINED_MAX_SLIPPAGE_PERCENT, SCALE_FACTOR }; // Version info for migration info @@ -48,25 +48,13 @@ pub fn instantiate( set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?; validate_input_tokens(&deps, &msg.token1155_denom, &msg.token2_denom)?; - - let token1155 = Token { - denom: msg.token1155_denom.clone(), - reserve: Uint128::zero(), - }; - TOKEN1155.save(deps.storage, &token1155)?; - - let token2 = Token { - denom: msg.token2_denom.clone(), - reserve: Uint128::zero(), - }; - TOKEN2.save(deps.storage, &token2)?; - - OWNER.save(deps.storage, &info.sender)?; - PENDING_OWNER.save(deps.storage, &None)?; + validate_fee_percent(msg.lp_fee_percent)?; + validate_fee_percent(msg.protocol_fee_percent)?; + validate_slippage_percent(msg.max_slippage_percent)?; let protocol_fee_recipient = deps.api.addr_validate(&msg.protocol_fee_recipient)?; let total_fee_percent = msg.lp_fee_percent + msg.protocol_fee_percent; - let max_fee_percent = Decimal::from_str(PREDEFINED_MAX_PERCENT)?; + let max_fee_percent = Decimal::from_str(PREDEFINED_MAX_FEES_PERCENT)?; if total_fee_percent > max_fee_percent { return Err(ContractError::FeesTooHigh { max_fee_percent, @@ -80,10 +68,23 @@ pub fn instantiate( protocol_fee_recipient, }; FEES.save(deps.storage, &fees)?; - - validate_slippage_percent(msg.max_slippage_percent)?; MAX_SLIPPAGE_PERCENT.save(deps.storage, &msg.max_slippage_percent)?; + let token1155 = Token { + denom: msg.token1155_denom.clone(), + reserve: Uint128::zero(), + }; + TOKEN1155.save(deps.storage, &token1155)?; + + let token2 = Token { + denom: msg.token2_denom.clone(), + reserve: Uint128::zero(), + }; + TOKEN2.save(deps.storage, &token2)?; + + OWNER.save(deps.storage, &info.sender)?; + PENDING_OWNER.save(deps.storage, &None)?; + // Depositing is not frozen by default FROZEN.save(deps.storage, &false)?; @@ -136,6 +137,26 @@ fn validate_slippage_percent(percent: Decimal) -> Result<(), ContractError> { Ok(()) } +/// Validates that fee percent is more than SCALE_FACTOR can handle, or zero +fn validate_fee_percent(percent: Decimal) -> Result<(), ContractError> { + if percent.is_zero() { + return Ok(()); + } + + let min_fee_percent = Decimal::from_str(MIN_FEE_PERCENT)?; + if percent < min_fee_percent { + return Err(ContractError::FeesTooLow { + min_fee_percent, + fee_percent: percent, + }); + } + + Ok(()) +} + +/// Validates the input tokens by: +/// - checking that the addresses are valid. +/// - checking that the token addresses are different. fn validate_input_tokens( deps: &DepsMut, token1155_denom: &Denom, @@ -292,6 +313,7 @@ pub fn execute( } } +/// Executes the `FreezeDeposits` message. fn execute_freeze_deposits( deps: DepsMut, sender: Addr, @@ -333,10 +355,8 @@ fn check_expiration( } /// Calculates the amount of lp token to mint by: -/// -/// 1 - if liquidity supply is zero then return token1_amount -/// -/// 2 - if liquidity supply is not zero then return token1_amount * liquidity_supply / token1_reserve +/// - if liquidity supply is zero then return token1_amount +/// - if liquidity supply is not zero then return token1_amount * liquidity_supply / token1_reserve fn get_lp_token_amount_to_mint( token1_amount: Uint128, liquidity_supply: Uint128, @@ -354,10 +374,8 @@ fn get_lp_token_amount_to_mint( } /// Calculates the amount of token2 required for adding to liquidity pool by: -/// -/// 1 - if liquidity supply is zero then return max_token -/// -/// 2 - if liquidity supply is not zero then return token1_amount * token2_reserve / token1_reserve + 1 +/// - if liquidity supply is zero then return max_token +/// - if liquidity supply is not zero then return token1_amount * token2_reserve / token1_reserve + 1 fn get_token2_amount_required( max_token: Uint128, token1_amount: Uint128, @@ -378,6 +396,7 @@ fn get_token2_amount_required( } } +/// Executes the `AddLiquidity` message. pub fn execute_add_liquidity( deps: DepsMut, info: &MessageInfo, @@ -564,9 +583,8 @@ fn validate_slippage( let max_slippage_percent = MAX_SLIPPAGE_PERCENT.load(deps.storage)?; let actual_token_decimal_amount = Decimal::from_str(actual_token_amount.to_string().as_str())?; - let min_required_decimal_amount = actual_token_decimal_amount - - (actual_token_decimal_amount * max_slippage_percent) - / Decimal::from_str(PREDEFINED_MAX_PERCENT)?; + let slippage_impact = actual_token_decimal_amount * max_slippage_percent; + let min_required_decimal_amount = actual_token_decimal_amount - slippage_impact; let min_required_amount = min_required_decimal_amount.to_uint_floor(); if min_token_amount < min_required_amount { @@ -586,11 +604,13 @@ fn get_cw1155_transfer_msg( token_addr: &Addr, tokens: &HashMap, ) -> Result { - // create transfer cw1155 msg + // Convert HashMap to BTreeMap to maintain deterministic order by key + let sorted_tokens: BTreeMap<_, _> = tokens.iter().collect(); + let transfer_cw1155_msg = Cw1155ExecuteMsg::BatchSendFrom { from: owner.into(), to: recipient.into(), - batch: tokens + batch: sorted_tokens .into_iter() .map(|(token_id, amount)| (token_id.clone(), amount.clone(), "".to_string())) .collect(), @@ -711,6 +731,7 @@ fn get_cw20_increase_allowance_msg( Ok(exec_allowance.into()) } +/// Executes the `TransferOwnership` message. pub fn execute_transfer_ownership( deps: DepsMut, info: MessageInfo, @@ -743,6 +764,7 @@ pub fn execute_transfer_ownership( Ok(Response::new().add_attributes(attributes)) } +/// Executes the `ClaimOwnership` message. pub fn execute_claim_ownership( deps: DepsMut, info: MessageInfo, @@ -766,6 +788,7 @@ pub fn execute_claim_ownership( Ok(Response::new().add_attributes(attributes)) } +/// Executes the `UpdateSlippage` message. pub fn execute_update_slippage( deps: DepsMut, info: MessageInfo, @@ -787,6 +810,7 @@ pub fn execute_update_slippage( ])) } +/// Executes the `UpdateFee` message. pub fn execute_update_fee( deps: DepsMut, info: MessageInfo, @@ -800,9 +824,12 @@ pub fn execute_update_fee( return Err(ContractError::Unauthorized {}); } + validate_fee_percent(lp_fee_percent)?; + validate_fee_percent(protocol_fee_percent)?; + // validate that total fee percent is less than max fee percent let total_fee_percent = lp_fee_percent + protocol_fee_percent; - let max_fee_percent = Decimal::from_str(PREDEFINED_MAX_PERCENT)?; + let max_fee_percent = Decimal::from_str(PREDEFINED_MAX_FEES_PERCENT)?; if total_fee_percent > max_fee_percent { return Err(ContractError::FeesTooHigh { max_fee_percent, @@ -827,6 +854,7 @@ pub fn execute_update_fee( ])) } +/// Executes the `RemoveLiquidity` message. pub fn execute_remove_liquidity( deps: DepsMut, info: MessageInfo, @@ -948,12 +976,18 @@ pub fn execute_remove_liquidity( ])) } -/// Gets a map of 1155 tokens to transfer based on the token1155_amount to transfer by: +/// Gets a map of 1155 tokens to transfer from TOKEN_SUPPLIES based on the token1155_amount to transfer by: +/// - if min_token1155 is single then it gets any random tokens from the TOKEN_SUPPLIES till amount is reached +/// - if min_token1155 is multiple then it: +/// - tries to get the min amount per token in the min_token1155, if there isn't then throw error +/// - if the min_token1155 didnt fill the needed token1155_amount, then loop through the min_token1155 ids +/// and get any remaining tokens left in supply for tokens ids, so to first fill the output tokens +/// with same token ids user defined in min_token1155 +/// - lastly if ther eis still remaining amount to be filled for transfer, then loop through the +/// TOKEN_SUPPLIES till get the amount of tokens wanted /// -/// 1 - if min_token1155 is single then it gets any random tokens from the TOKEN_SUPPLIES till amount is reached -/// -/// 2 - if min_token1155 is multiple then it gets the amount of tokens from the TOKEN_SUPPLIES that matches the multiple -/// inputs, otherwise it throws and error if the amount of tokens from the TOKEN_SUPPLIES is less than the amount +/// NOTE: this assumes that token1155_amount is >= to the total of the min_token1155 amount +/// Please ensure this assumtion is kept by validations before calling this function fn get_token_amounts_to_transfer( storage: &mut dyn Storage, token1155_amount: Uint128, @@ -964,8 +998,13 @@ fn get_token_amounts_to_transfer( match min_token1155 { TokenAmount::Multiple(amounts) => { + // local cache map of tokens that has remaining supply after the min_token1155 is subtracted let mut token1155_supplies: HashMap = HashMap::new(); + // map over min_token1155 and per token: + // - check if the token supply is less than the amount, if so return error + // - subtract the amount from the token1155_amount_left_to_transfer + // - update the TOKEN_SUPPLIES and token1155_supplies with the remaining supply for the specific token for (token_id, token_amount) in amounts.into_iter() { let token_supply = TOKEN_SUPPLIES .may_load(storage, token_id.clone())? @@ -981,63 +1020,72 @@ fn get_token_amounts_to_transfer( token1155_amount_left_to_transfer -= token_amount; token1155_amounts_to_transfer.insert(token_id.clone(), token_amount); - let remaining_supply = token_supply - token_amount; update_token_supplies( storage, - remaining_supply, + token_supply - token_amount, token_id, Some(&mut token1155_supplies), )?; } - while !token1155_amount_left_to_transfer.is_zero() && !token1155_supplies.is_empty() { - let additional_amount_to_transfer = token1155_amount_left_to_transfer - .checked_div(Uint128::from(token1155_supplies.len() as u32)) - .map_err(StdError::divide_by_zero)?; + // if there is still has an amount left to transfer, then first loop through the token1155_supplies local + // cache map, so that first try and fill remaining amount with same tokens as min_token1155 + if !token1155_amount_left_to_transfer.is_zero() { + let mut sorted_supplies: Vec<(TokenId, Uint128)> = token1155_supplies.into_iter().collect(); + // Sort by amount (ascending), then by token_id for deterministic order + sorted_supplies.sort_by(|a, b| { + if a.1 == b.1 { + a.0.cmp(&b.0) + } else { + a.1.cmp(&b.1) + } + }); - for (token_id, token_amount) in token1155_supplies.clone().into_iter() { + for (token_id, token_supply) in sorted_supplies.into_iter() { if token1155_amount_left_to_transfer.is_zero() { break; } - let mut optional_additional_amount = None; - let mut optional_supplies = None; - if !additional_amount_to_transfer.is_zero() { - optional_additional_amount = Some(additional_amount_to_transfer); - optional_supplies = Some(&mut token1155_supplies); + let take_amount = if token_supply >= token1155_amount_left_to_transfer { + token1155_amount_left_to_transfer + } else { + token_supply }; - update_token_amounts( + token1155_amount_left_to_transfer -= take_amount; + *token1155_amounts_to_transfer.entry(token_id.clone()).or_insert(Uint128::zero()) += take_amount; + + update_token_supplies( storage, - &mut token1155_amounts_to_transfer, - &mut token1155_amount_left_to_transfer, + token_supply - take_amount, token_id, - token_amount, - optional_additional_amount, - optional_supplies, + None, )?; } } + + // lastly while there is still an amount left to transfer, we run the process_token_supplies_in_chunks to get + // the amounts from any tokens in the TOKEN_SUPPLIES that are left to transfer + let res = process_token_supplies_in_chunks( + storage, + &mut token1155_amounts_to_transfer, + &mut token1155_amount_left_to_transfer, + token1155_amount, + ); + if let Err(err) = res { + return Err(err); + } } + // runs the process_token_supplies_in_chunks to get the amounts from any tokens in the TOKEN_SUPPLIES to transfer TokenAmount::Single(_) => { - let token_supplies = TOKEN_SUPPLIES - .range(storage, None, None, Order::Ascending) - .collect::>>()?; - - let mut supply_index = 0; - while !token1155_amount_left_to_transfer.is_zero() { - let (token_id, token_amount) = token_supplies[supply_index].clone(); - - update_token_amounts( - storage, - &mut token1155_amounts_to_transfer, - &mut token1155_amount_left_to_transfer, - token_id, - token_amount, - None, - None, - )?; - supply_index += 1; + let res = process_token_supplies_in_chunks( + storage, + &mut token1155_amounts_to_transfer, + &mut token1155_amount_left_to_transfer, + token1155_amount, + ); + if let Err(err) = res { + return Err(err); } } }; @@ -1045,6 +1093,65 @@ fn get_token_amounts_to_transfer( Ok(token1155_amounts_to_transfer.clone()) } +/// Gets the tokens to transfer by processes the TOKEN_SUPPLIES in chunks of 100 at a time, and updates the +/// mutable token1155_amounts_to_transfer and mutable token1155_amount_left_to_transfer passed by runnig the +/// update_token_amounts per token. see update_token_amounts for more details. +/// Batching of 100 is done to not load the whole TOKEN_SUPPLIES into memory +fn process_token_supplies_in_chunks( + storage: &mut dyn Storage, + token1155_amounts_to_transfer: &mut HashMap, + token1155_amount_left_to_transfer: &mut Uint128, + original_token1155_amount: Uint128, +) -> Result<(), ContractError> { + while !token1155_amount_left_to_transfer.is_zero() { + let token_supplies = TOKEN_SUPPLIES + .range(storage, None, None, Order::Ascending) + .take(100) // Process in chunks of 100 entries + .collect::>>()?; + + // this should never happen, but just in case + if token_supplies.is_empty() { + return Err(ContractError::InsufficientTokenSupply { + requested: original_token1155_amount, + available: original_token1155_amount - *token1155_amount_left_to_transfer, + }); + } + + for (token_id, token_amount) in token_supplies.into_iter() { + update_token_amounts( + storage, + token1155_amounts_to_transfer, + token1155_amount_left_to_transfer, + token_id.clone(), + token_amount, + None, + None, + )?; + + if token1155_amount_left_to_transfer.is_zero() { + break; + } + } + } + + Ok(()) +} + +/// Updates the mutable token_amounts_to_transfer and mutable token_amount_left_to_transfer passed by: +/// - getting the tokens current amount from the token_amounts_to_transfer, aka that is already in map to transfer +/// - if the passed token amount is less than the wanted amount, we: +/// - add the tokens remaining amount to the amount to transfer +/// - make remaining supply zero for the specific token +/// - subtract the taken amount from mutable token_amount_left_to_transfer for next loop iteration +/// - if the passed token amount is >= the wanted amount, we: +/// - add wanted amount to the amount to transfer +/// - subtract the amount we added from the remaining supply for the specific token +/// - if additional_token_amount_to_transfer is none, we set mutable token_amount_left_to_transfer to zero since +/// all the wanted token amount is accounted for then +/// - if additional_token_amount_to_transfer is some, we subtract the additional token amount to transfer from the mutable +/// - then it updates the mutable token_amounts_to_transfer with the new amount to transfer +/// - lastly it updates the TOKEN_SUPPLIES with the remaining supply for the specific token, as well as the mutable +/// token_supplies if provided, see function update_token_supplies for more details fn update_token_amounts( storage: &mut dyn Storage, token_amounts_to_transfer: &mut HashMap, @@ -1054,16 +1161,23 @@ fn update_token_amounts( additional_token_amount_to_transfer: Option, token_supplies: Option<&mut HashMap>, ) -> StdResult<()> { + // get the current tokens amount that is already in the token_amounts_to_transfer let mut amount_to_transfer = *token_amounts_to_transfer .get(&token_id) .unwrap_or(&Uint128::zero()); + // token_supply is the amount of tokens that we ideally want to transfer and add to token_amounts_to_transfer let mut token_supply = *token_amount_left_to_transfer; if let Some(token_amount) = additional_token_amount_to_transfer { token_supply = token_amount; } let remaining_supply; + // if token amount is >= the amount we want, we: + // - add wanted amount to the amount to transfer + // - subtract the amount we want from the remaining supply for the specific token + // - if additional_token_amount_to_transfer is none, we set mutable token_amount_left_to_transfer to zero since this + // if took the amount needed, otherwise we subtract the additional token amount to transfer from the mutable if token_amount >= token_supply { amount_to_transfer += token_supply; remaining_supply = token_amount - token_supply; @@ -1074,6 +1188,10 @@ fn update_token_amounts( *token_amount_left_to_transfer = Uint128::zero(); } } else { + // if token amount is < the amount we want, we: + // - add all the tokens remaining amount to the amount to transfer + // - make remaining supply zero for the specific token + // - subtract the taken amount from mutable token_amount_left_to_transfer for next loop iteration amount_to_transfer += token_amount; remaining_supply = Uint128::zero(); *token_amount_left_to_transfer -= token_amount; @@ -1083,7 +1201,11 @@ fn update_token_amounts( update_token_supplies(storage, remaining_supply, token_id.clone(), token_supplies) } -/// Updates the TOKEN_SUPPLIES by subtracting the amount of tokens from the total supply and removing the token id if it is zero +/// Updates the TOKEN_SUPPLIES, and mutable token_supplies(if it is provided) passed by: +/// - if the remaining supply is zero, we remove the token id from the TOKEN_SUPPLIES and token_supplies +/// - otherwise we update the supply for the token id in the TOKEN_SUPPLIES and token_supplies +/// +/// NOTE: the token_supplies is mutable so the passed in token_supplies will be updated if provided fn update_token_supplies( storage: &mut dyn Storage, remaining_supply: Uint128, @@ -1178,16 +1300,11 @@ fn get_fee_transfer_msg( } /// Calculates the amount of tokens the user bought by:. -/// -/// 1 - if either reserve is zero throws error -/// -/// 2 - create fee percent with SCALE_FACTOR and calculate input_amount_with_fee -/// -/// 3 - calculate numerator: input_amount_with_fee * output_reserve -/// -/// 4 - calculate denominator: input_reserve * SCALE_FACTOR + input_amount_with_fee -/// -/// 5 - calculate amount bought: numerator / denominator +/// - if either reserve is zero throws error +/// - create fee percent with SCALE_FACTOR and calculate input_amount_with_fee +/// - calculate numerator: input_amount_with_fee * output_reserve +/// - calculate denominator: input_reserve * SCALE_FACTOR + input_amount_with_fee +/// - calculate amount bought: numerator / denominator fn get_input_price( input_amount: Uint128, input_reserve: Uint128, @@ -1217,12 +1334,11 @@ fn get_input_price( } /// Calculates the amount of tokens the reserves get, aka input amount minus the protocol fees, by: +/// - if fee amount is none then return input amount +/// - if fee amount is some then return input amount - fee amount /// -/// 1 - if fee amount is none then return input amount -/// -/// 2 - if fee amount is some then return input amount - fee amount /// For single token input amount, it is input amount - fee amount -/// For multiple token input amount, it is input amount - fee amount for each token +/// For multiple token input amount, it is input amount - fee amount if it exists in the fee amount per token fn get_amount_without_fee( input_amount: &TokenAmount, fee_amount: Option, @@ -1232,8 +1348,11 @@ fn get_amount_without_fee( TokenAmount::Multiple(mut input_amounts) => { let fee_amounts = fee_amount.get_multiple()?; + let zero = Uint128::zero(); for (token_id, token_amount) in input_amounts.iter_mut() { - let fee_amount = fee_amounts.get(token_id).unwrap(); + // it is possible that there are some tokens that is not in the fee_amount, due to the way it is calculated + // refer to get_percent_from_multiple for more details + let fee_amount = fee_amounts.get(token_id).unwrap_or(&zero); *token_amount -= fee_amount } @@ -1251,6 +1370,7 @@ fn get_amount_without_fee( } } +// Executes the `Swap` message. #[allow(clippy::too_many_arguments)] pub fn execute_swap( deps: DepsMut, @@ -1286,9 +1406,19 @@ pub fn execute_swap( let input_amount_total = input_amount.get_total(); let fees = FEES.load(deps.storage)?; + + // can do early validation, if protocol fee is not 0 and input == 1, then can throw error already, since + // protocol fees are rounded up so it will be minimum 1 + if fees.protocol_fee_percent > Decimal::zero() && input_amount_total == Uint128::one() { + return Err(ContractError::MinInputTokenAmountError { + input_token_amount: input_amount_total, + min_allowed: Uint128::from_str("2")?, + }); + } + let total_fee_percent = fees.lp_fee_percent + fees.protocol_fee_percent; // get the calculated amount of tokens bought - let mut token_bought = get_input_price( + let token_bought = get_input_price( input_amount_total, input_token.reserve, output_token.reserve, @@ -1328,8 +1458,10 @@ pub fn execute_swap( _ => {} }; + let mut protocol_fee_amount_total = Uint128::zero(); // Send protocol fee to protocol fee recipient if let Some(protocol_fee_amount) = protocol_fee_amount { + protocol_fee_amount_total = protocol_fee_amount.get_total(); msgs.push(get_fee_transfer_msg( &info.sender, &fees.protocol_fee_recipient, @@ -1344,7 +1476,6 @@ pub fn execute_swap( Denom::Cw1155(addr, _) => { let tokens_to_transfer = get_token_amounts_to_transfer(deps.storage, token_bought, min_token)?; - token_bought = TokenAmount::Multiple(tokens_to_transfer.clone()).get_total(); get_cw1155_transfer_msg( &env.contract.address, @@ -1408,6 +1539,7 @@ pub fn execute_swap( attr("input_token_enum", input_token_enum.to_string()), attr("input_token_amount", input_amount_total), attr("output_token_amount", token_bought), + attr("protocol_fee_amount", protocol_fee_amount_total), ]; // Add updated reserves based on the token type @@ -1426,6 +1558,7 @@ pub fn execute_swap( Ok(Response::new().add_messages(msgs).add_attributes(attributes)) } +// Executes the `PassThroughSwap` message. #[allow(clippy::too_many_arguments)] pub fn execute_pass_through_swap( deps: DepsMut, @@ -1466,12 +1599,23 @@ pub fn execute_pass_through_swap( &info.sender, )?; - let input_token_amount_total = input_token_amount.get_total(); + let input_amount_total = input_token_amount.get_total(); let fees = FEES.load(deps.storage)?; + + // can do early validation, if protocol fee is not 0 and input == 1, then can throw error already, since + // protocol fees are rounded up so it will be minimum 1 + if fees.protocol_fee_percent > Decimal::zero() && input_amount_total == Uint128::one() { + return Err(ContractError::MinInputTokenAmountError { + input_token_amount: input_amount_total, + min_allowed: Uint128::from_str("2")?, + }); + } + + let total_fee_percent = fees.lp_fee_percent + fees.protocol_fee_percent; // get the calculated amount of tokens bought let amount_to_transfer = get_input_price( - input_token_amount_total, + input_amount_total, input_token.reserve, transfer_token.reserve, total_fee_percent, @@ -1500,8 +1644,10 @@ pub fn execute_pass_through_swap( _ => {} }; + let mut protocol_fee_amount_total = Uint128::zero(); // Send protocol fee to protocol fee recipient if let Some(protocol_fee_amount) = protocol_fee_amount { + protocol_fee_amount_total = protocol_fee_amount.get_total(); msgs.push(get_fee_transfer_msg( &info.sender, &fees.protocol_fee_recipient, @@ -1602,10 +1748,11 @@ pub fn execute_pass_through_swap( let mut attributes = vec![ attr("action", "cross-contract-swap"), attr("input_token_enum", input_token_enum.to_string()), - attr("input_token_amount", input_token_amount_total), + attr("input_token_amount", input_amount_total), attr("output_token_amount", amount_to_transfer), attr("output_amm_address", output_amm_address.to_string()), attr("recipient", info.sender.to_string()), + attr("protocol_fee_amount", protocol_fee_amount_total), ]; // Add updated reserves based on the token type @@ -1623,6 +1770,7 @@ pub fn execute_pass_through_swap( Ok(Response::new().add_messages(msgs).add_attributes(attributes)) } +// Queries for the contract state. #[cfg_attr(not(feature = "library"), entry_point)] pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { match msg { @@ -1643,6 +1791,7 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { } } +// Queries for token module token metadata. pub fn query_token_metadata(deps: Deps, id: String) -> StdResult { deps.querier.query(&QueryRequest::Stargate { path: "/ixo.token.v1beta1.Query/TokenMetadata".to_string(), @@ -1650,6 +1799,7 @@ pub fn query_token_metadata(deps: Deps, id: String) -> StdResult StdResult { deps.querier.query(&QueryRequest::Stargate { path: "/cosmos.bank.v1beta1.Query/DenomMetadata".to_string(), @@ -1918,22 +2068,31 @@ mod tests { fn should_return_token_amounts_to_transfer_when_multiple_input_token_provided() { let mut deps = mock_dependencies(); - let token_ids = vec!["1".to_string(), "2".to_string(), "3".to_string()]; - let mut token_amounts = HashMap::new(); + // let token_ids = vec!["1".to_string(), "2".to_string(), "3".to_string()]; + // let mut token_amounts = HashMap::new(); let mut token_supplies = HashMap::new(); - for token_id in token_ids { - token_amounts.insert(token_id.clone(), Uint128::new(100)); - token_supplies.insert(token_id.clone(), Uint128::new(1000)); + let mut tokens = HashMap::new(); + tokens.insert("1".to_string(), Uint128::new(500)); + tokens.insert("2".to_string(), Uint128::new(500)); + tokens.insert("3".to_string(), Uint128::new(500)); + + let mut min_token_amounts = HashMap::new(); + min_token_amounts.insert("1".to_string(), Uint128::new(100)); + + // map through tokens and add to token_supplies + for (token_id, token_amount) in tokens.clone().into_iter() { + // token_amounts.insert(token_id.clone(), Uint128::new(100)); + token_supplies.insert(token_id.clone(), token_amount); TOKEN_SUPPLIES - .save(&mut deps.storage, token_id.clone(), &Uint128::new(1000)) + .save(&mut deps.storage, token_id.clone(), &token_amount) .unwrap(); } let token_amounts_to_transfer = get_token_amounts_to_transfer( &mut deps.storage, - Uint128::new(500), - TokenAmount::Multiple(token_amounts.clone()), + Uint128::new(501), + TokenAmount::Multiple(min_token_amounts.clone()), ) .unwrap(); @@ -1944,12 +2103,12 @@ mod tests { ); assert_eq!( TokenAmount::Multiple(token_amounts_to_transfer).get_total(), - Uint128::new(500) + Uint128::new(501) ) } #[test] - fn should_return_partial_token_amounts_to_transfer_when_multiple_input_token_provided() { + fn should_fail_token_amounts_to_transfer_is_less_than_supply() { let mut deps = mock_dependencies(); let token_ids = vec!["1".to_string(), "2".to_string(), "3".to_string()]; @@ -1972,17 +2131,15 @@ mod tests { Uint128::new(1000), TokenAmount::Multiple(token_amounts.clone()), ) - .unwrap(); + .unwrap_err(); - assert_token_supplies( - &mut deps.storage, - &token_amounts_to_transfer, - &token_supplies, - ); assert_eq!( - TokenAmount::Multiple(token_amounts_to_transfer).get_total(), - Uint128::new(600) - ) + token_amounts_to_transfer, + ContractError::InsufficientTokenSupply { + requested: Uint128::new(1_000), + available: Uint128::new(600) + } + ); } #[test] @@ -2047,7 +2204,7 @@ mod tests { .save(&mut deps.storage, &Decimal::from_str("0.1").unwrap()) .unwrap(); - let min_token_amount = Uint128::new(105_000); + let min_token_amount = Uint128::new(108_000); let actual_token_amount = Uint128::new(110_000); let res = validate_slippage(&deps.as_mut(), min_token_amount, actual_token_amount).unwrap(); @@ -2055,8 +2212,8 @@ mod tests { } #[test] - fn should_fail_slippage_percent_validation_when_percent_greater_than_50() { - let percent = Decimal::from_str("0.6").unwrap(); + fn should_fail_slippage_percent_validation_when_slippage_too_high() { + let percent = Decimal::from_str("10.1").unwrap(); let err = validate_slippage_percent(percent).unwrap_err(); assert_eq!( err, diff --git a/ixo-swap/src/error.rs b/ixo-swap/src/error.rs index 87b5ee4..50c52d3 100644 --- a/ixo-swap/src/error.rs +++ b/ixo-swap/src/error.rs @@ -30,6 +30,12 @@ pub enum ContractError { tokens_required: Uint128, }, + #[error("Min input token amount error: input_token_amount: {input_token_amount}, min_allowed {min_allowed}")] + MinInputTokenAmountError { + input_token_amount: Uint128, + min_allowed: Uint128, + }, + #[error("Min token amount error: requested: {min_token}, min_required {min_required}")] MinTokenAmountError { min_token: Uint128, @@ -45,6 +51,12 @@ pub enum ContractError { available: Uint128, }, + #[error("Insufficient token supply error: requested: {requested}, available: {available}")] + InsufficientTokenSupply { + requested: Uint128, + available: Uint128, + }, + #[error("Min token1155 error: requested: {requested}, available: {available}")] MinToken1155Error { requested: Uint128, @@ -69,6 +81,12 @@ pub enum ContractError { total_fee_percent: Decimal, }, + #[error("Fee ({fee_percent}) percent is lower than allowed min ({min_fee_percent})")] + FeesTooLow { + min_fee_percent: Decimal, + fee_percent: Decimal, + }, + #[error("InsufficientFunds")] InsufficientFunds {}, diff --git a/ixo-swap/src/integration_test.rs b/ixo-swap/src/integration_test.rs index dfcec78..94f0069 100644 --- a/ixo-swap/src/integration_test.rs +++ b/ixo-swap/src/integration_test.rs @@ -23,6 +23,7 @@ use crate::msg::{ TokenSuppliesResponse, }; use crate::token_amount::TokenAmount; +use crate::utils::{MIN_FEE_PERCENT, PREDEFINED_MAX_FEES_PERCENT}; use crate::{error::ContractError, msg::Denom}; #[derive(Clone)] @@ -297,7 +298,7 @@ fn instantiate() { let max_slippage_percent = Decimal::from_str("0.3").unwrap(); let supported_denom = "CARBON".to_string(); - let lp_fee_percent = Decimal::from_str("0.3").unwrap(); + let lp_fee_percent = Decimal::from_str("0.01").unwrap(); let protocol_fee_percent = Decimal::zero(); // instantiate @@ -388,6 +389,30 @@ fn instantiate() { err.downcast().unwrap() ); + // instantiate with fee < 0.01% should fail + let cw20_id = router.store_code(contract_cw20()); + let amm_id = router.store_code(contract_amm()); + let low_protocol_fee = Decimal::from_str("0.001").unwrap(); + let msg = InstantiateMsg { + token1155_denom: Denom::Cw1155(cw1155_token.clone(), supported_denom.clone()), + token2_denom: Denom::Native(NATIVE_TOKEN_DENOM.into()), + lp_token_code_id: cw20_id, + max_slippage_percent, + lp_fee_percent, + protocol_fee_percent: low_protocol_fee, + protocol_fee_recipient: owner.to_string(), + }; + let err = router + .instantiate_contract(amm_id, owner.clone(), &msg, &[], "amm", None) + .unwrap_err(); + assert_eq!( + ContractError::FeesTooLow { + min_fee_percent: Decimal::from_str(MIN_FEE_PERCENT).unwrap(), + fee_percent: low_protocol_fee + }, + err.downcast().unwrap() + ); + // try instantiate with invalid token address let cw20_id = router.store_code(contract_cw20()); let amm_id = router.store_code(contract_amm()); @@ -440,7 +465,7 @@ fn instantiate() { assert_eq!(ContractError::InvalidTokenType {}, err.downcast().unwrap()); // try instantiate with invalid fee amount - let lp_fee_percent = Decimal::from_str("1.01").unwrap(); + let lp_fee_percent = Decimal::from_str("5.01").unwrap(); let protocol_fee_percent = Decimal::zero(); let cw20_id = router.store_code(contract_cw20()); let amm_id = router.store_code(contract_amm()); @@ -458,8 +483,8 @@ fn instantiate() { .unwrap_err(); assert_eq!( ContractError::FeesTooHigh { - max_fee_percent: Decimal::from_str("1").unwrap(), - total_fee_percent: Decimal::from_str("1.01").unwrap() + max_fee_percent: Decimal::from_str(PREDEFINED_MAX_FEES_PERCENT).unwrap(), + total_fee_percent: Decimal::from_str("5.01").unwrap() }, err.downcast().unwrap() ); @@ -673,7 +698,7 @@ fn cw1155_to_cw1155_swap() { .unwrap(); let event = Event::new("wasm").add_attributes(vec![ attr("action", "cross-contract-swap"), - attr("input_token_enum", "Token1155"), + attr("input_token_enum", "token1155"), attr("input_token_amount", Uint128::new(50)), attr("output_token_amount", Uint128::new(33)), attr("output_amm_address", amm2.to_string()), @@ -874,15 +899,17 @@ fn cw1155_to_cw20_swap() { let res = router .execute_contract(owner.clone(), amm.clone(), &swap_msg, &[]) .unwrap(); + println!("res: {:?}", res); let event = Event::new("wasm").add_attributes(vec![ attr("action", "swap"), attr("sender", owner.clone()), attr("recipient", owner.clone()), - attr("input_token_enum", "Token1155"), + attr("input_token_enum", "token1155"), attr("input_token_amount", Uint128::new(50_000)), attr("output_token_amount", Uint128::new(33_266)), attr("token1155_reserve", Uint128::new(149950)), // prev amount(100_000) plus added amount(50_000) - minus fees(50) attr("token2_reserve", Uint128::new(66_734)), // prev amount(100_000) minus removed amount(33_266) + attr("protocol_fee_amount", Uint128::new(50)), ]); assert!(res.has_event(&event)); @@ -895,7 +922,7 @@ fn cw1155_to_cw20_swap() { let fee_recipient_balance = batch_balance_for_owner(&router, &cw1155_token, &protocol_fee_recipient, &token_ids) .balances; - assert_eq!(fee_recipient_balance, [Uint128::new(25), Uint128::new(25)]); + assert_eq!(fee_recipient_balance, [Uint128::new(50), Uint128::new(0)]); // Swap cw20 for cw1155 let allowance_msg = Cw20ExecuteMsg::IncreaseAllowance { @@ -923,7 +950,7 @@ fn cw1155_to_cw20_swap() { // ensure balances updated let owner_balance = batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; - assert_eq!(owner_balance, [Uint128::new(60_439), Uint128::new(60_439)]); + assert_eq!(owner_balance, [Uint128::new(65_878), Uint128::new(55_000)]); let owner_balance = cw20_token.balance(&router.wrap(), owner.clone()).unwrap(); assert_eq!(owner_balance, Uint128::new(23_266)); let fee_recipient_balance = cw20_token @@ -1053,7 +1080,7 @@ fn cw1155_to_native_swap() { let fee_recipient_balance = batch_balance_for_owner(&router, &cw1155_token, &protocol_fee_recipient, &token_ids) .balances; - assert_eq!(fee_recipient_balance, [Uint128::new(25), Uint128::new(25)]); + assert_eq!(fee_recipient_balance, [Uint128::new(50), Uint128::new(0)]); // Swap native for cw1155 let swap_msg = ExecuteMsg::Swap { @@ -1080,7 +1107,7 @@ fn cw1155_to_native_swap() { // ensure balances updated let owner_balance = batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; - assert_eq!(owner_balance, [Uint128::new(60_439), Uint128::new(60_439)]); + assert_eq!(owner_balance, [Uint128::new(65_878), Uint128::new(55_000)]); let owner_balance: Coin = bank_balance(&mut router, &owner, NATIVE_TOKEN_DENOM.to_string()); assert_eq!(owner_balance.amount, Uint128::new(23_266)); let fee_recipient_balance = bank_balance( @@ -1091,6 +1118,197 @@ fn cw1155_to_native_swap() { assert_eq!(fee_recipient_balance.amount, Uint128::new(60)); } +#[test] +fn cw1155_to_native_swap_low_fees() { + let mut router = mock_app(); + + const NATIVE_TOKEN_DENOM: &str = "juno"; + + let owner = Addr::unchecked("owner"); + let protocol_fee_recipient = Addr::unchecked("protocol_fee_recipient"); + + let funds = coins(150_000, NATIVE_TOKEN_DENOM); + router.borrow_mut().init_modules(|router, _, storage| { + router.bank.init_balance(storage, &owner, funds).unwrap(); + router.stargate.register_query( + "/ixo.token.v1beta1.Query/TokenMetadata", + Box::new(TokenMetadataQueryHandler), + ); + router.stargate.register_query( + "/cosmos.bank.v1beta1.Query/DenomMetadata", + Box::new(DenomMetadataQueryHandler), + ) + }); + + let cw1155_token = create_cw1155(&mut router, &owner); + let token_ids = vec![TokenId::from("FIRST/1"), TokenId::from("FIRST/2")]; + + let max_slippage_percent = Decimal::from_str("0.3").unwrap(); + + let lp_fee_percent = Decimal::from_str("0.0").unwrap(); + let protocol_fee_percent = Decimal::from_str("0.01").unwrap(); + + let amm = create_amm( + &mut router, + &owner, + Denom::Cw1155(cw1155_token.clone(), "FIRST".to_string()), + Denom::Native(NATIVE_TOKEN_DENOM.into()), + max_slippage_percent, + lp_fee_percent, + protocol_fee_percent, + protocol_fee_recipient.to_string(), + ); + + // set up initial balances + let mint_msg = Cw1155ExecuteMsg::BatchMint { + to: owner.clone().into(), + batch: vec![ + (token_ids[0].clone(), Uint128::new(100_000), "".to_string()), + (token_ids[1].clone(), Uint128::new(100_000), "".to_string()), + ], + msg: None, + }; + let _res = router + .execute_contract(owner.clone(), cw1155_token.clone(), &mint_msg, &[]) + .unwrap(); + + // check initial balances + let owner_balance = batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids); + assert_eq!( + owner_balance.balances, + [Uint128::new(100_000), Uint128::new(100_000),] + ); + + // send tokens to contract address + let allowance_msg = Cw1155ExecuteMsg::ApproveAll { + operator: amm.clone().into(), + expires: None, + }; + let _res = router + .execute_contract(owner.clone(), cw1155_token.clone(), &allowance_msg, &[]) + .unwrap(); + + let add_liquidity_msg = ExecuteMsg::AddLiquidity { + token1155_amounts: HashMap::from([ + (token_ids[0].clone(), Uint128::new(50_000)), + (token_ids[1].clone(), Uint128::new(50_000)), + ]), + min_liquidity: Uint128::new(100_000), + max_token2: Uint128::new(100_000), + expiration: None, + }; + let _res = router + .execute_contract( + owner.clone(), + amm.clone(), + &add_liquidity_msg, + &[Coin { + denom: NATIVE_TOKEN_DENOM.into(), + amount: Uint128::new(100_000), + }], + ) + .unwrap(); + + // ensure balances updated + let owner_balance = + batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; + assert_eq!(owner_balance, [Uint128::new(50_000), Uint128::new(50_000)]); + let owner_balance: Coin = bank_balance(&mut router, &owner, NATIVE_TOKEN_DENOM.to_string()); + assert_eq!(owner_balance.amount, Uint128::new(50_000)); + + // Swap cw1155 for native + let swap_msg = ExecuteMsg::Swap { + input_token: TokenSelect::Token1155, + input_amount: TokenAmount::Multiple(HashMap::from([ + (token_ids[0].clone(), Uint128::new(5_000)), + (token_ids[1].clone(), Uint128::new(5_000)), + ])), + min_output: TokenAmount::Single(Uint128::new(6_500)), + expiration: None, + }; + let _res = router + .execute_contract(owner.clone(), amm.clone(), &swap_msg, &[]) + .unwrap(); + + // ensure balances updated + let owner_balance = + batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; + assert_eq!(owner_balance, [Uint128::new(45_000), Uint128::new(45_000)]); + let owner_balance: Coin = bank_balance(&mut router, &owner, NATIVE_TOKEN_DENOM.to_string()); + assert_eq!(owner_balance.amount, Uint128::new(59_090)); + let fee_recipient_balance = + batch_balance_for_owner(&router, &cw1155_token, &protocol_fee_recipient, &token_ids) + .balances; + // should be 1 "FIRST/1" since it alphabetically comes first and with very low protocol fee low inout amount the fee is rounded up to 1 + assert_eq!(fee_recipient_balance, [Uint128::new(1), Uint128::new(0)]); + + // Swap native for cw1155 + let swap_msg = ExecuteMsg::Swap { + input_token: TokenSelect::Token2, + input_amount: TokenAmount::Single(Uint128::new(7_000)), + min_output: TokenAmount::Multiple(HashMap::from([ + (token_ids[0].clone(), Uint128::new(3_000)), + (token_ids[1].clone(), Uint128::new(3_000)), + ])), + expiration: None, + }; + let _res: cw_multi_test::AppResponse = router + .execute_contract( + owner.clone(), + amm.clone(), + &swap_msg, + &[Coin { + denom: NATIVE_TOKEN_DENOM.into(), + amount: Uint128::new(7_000), + }], + ) + .unwrap(); + + // ensure balances updated + let owner_balance = + batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; + assert_eq!(owner_balance, [Uint128::new(49_863), Uint128::new(48_000)]); + let owner_balance: Coin = bank_balance(&mut router, &owner, NATIVE_TOKEN_DENOM.to_string()); + assert_eq!(owner_balance.amount, Uint128::new(52_090)); + let fee_recipient_balance = bank_balance( + &mut router, + &protocol_fee_recipient, + NATIVE_TOKEN_DENOM.to_string(), + ); + // since very low protocol fee and low input amount the fee is rounded up to 1 + assert_eq!(fee_recipient_balance.amount, Uint128::new(1)); + + + // Swap input 1 should fail since protocol fee is not zero + let swap_msg = ExecuteMsg::Swap { + input_token: TokenSelect::Token2, + input_amount: TokenAmount::Single(Uint128::new(1)), + min_output: TokenAmount::Multiple(HashMap::from([ + (token_ids[0].clone(), Uint128::new(1)), + ])), + expiration: None, + }; + let err = router + .execute_contract( + owner.clone(), + amm.clone(), + &swap_msg, + &[Coin { + denom: NATIVE_TOKEN_DENOM.into(), + amount: Uint128::new(1), + }], + ) + .unwrap_err(); + + assert_eq!( + ContractError::MinInputTokenAmountError { + input_token_amount: Uint128::new(1), + min_allowed: Uint128::new(2), + }, + err.downcast().unwrap() + ); +} + #[test] // receive cw20 tokens and release upon approval fn amm_add_and_remove_liquidity() { @@ -1523,12 +1741,12 @@ fn amm_add_and_remove_liquidity() { // ensure balances updated let owner_balance = batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; - assert_eq!(owner_balance, [Uint128::new(4920), Uint128::new(4980)]); + assert_eq!(owner_balance, [Uint128::new(4915), Uint128::new(4985)]); let token_supplies = get_owner_lp_tokens_balance(&router, &amm_addr, &token_ids).supplies; - assert_eq!(token_supplies, [Uint128::new(80), Uint128::new(20)]); + assert_eq!(token_supplies, [Uint128::new(85), Uint128::new(15)]); let amm_balances = batch_balance_for_owner(&router, &cw1155_token, &amm_addr, &token_ids).balances; - assert_eq!(amm_balances, [Uint128::new(80), Uint128::new(20)]); + assert_eq!(amm_balances, [Uint128::new(85), Uint128::new(15)]); let crust_balance = lp_token.balance(&router.wrap(), owner.clone()).unwrap(); assert_eq!(crust_balance, Uint128::new(100)); @@ -1545,8 +1763,8 @@ fn amm_add_and_remove_liquidity() { let remove_liquidity_msg = ExecuteMsg::RemoveLiquidity { amount: Uint128::new(100), min_token1155: TokenAmount::Multiple(HashMap::from([ - (token_ids[0].clone(), Uint128::new(80)), - (token_ids[1].clone(), Uint128::new(20)), + (token_ids[0].clone(), Uint128::new(85)), + (token_ids[1].clone(), Uint128::new(15)), ])), min_token2: Uint128::new(100), expiration: None, @@ -1738,7 +1956,7 @@ fn remove_liquidity_with_partially_and_any_filling() { [ Uint128::new(5000), Uint128::new(5000), - Uint128::new(4950), + Uint128::new(4955), Uint128::new(4990) ] ); @@ -1748,7 +1966,7 @@ fn remove_liquidity_with_partially_and_any_filling() { [ Uint128::new(0), Uint128::new(0), - Uint128::new(50), + Uint128::new(45), Uint128::new(10) ] ); @@ -1759,7 +1977,7 @@ fn remove_liquidity_with_partially_and_any_filling() { [ Uint128::new(0), Uint128::new(0), - Uint128::new(50), + Uint128::new(45), Uint128::new(10) ] ); @@ -1795,7 +2013,7 @@ fn remove_liquidity_with_partially_and_any_filling() { Uint128::new(5000), Uint128::new(5000), Uint128::new(5000), - Uint128::new(4995) + Uint128::new(5000) ] ); let token_supplies = get_owner_lp_tokens_balance(&router, &amm_addr, &token_ids).supplies; @@ -1805,7 +2023,7 @@ fn remove_liquidity_with_partially_and_any_filling() { Uint128::new(0), Uint128::new(0), Uint128::new(0), - Uint128::new(5) + Uint128::new(0) ] ); let amm_balances = @@ -1816,7 +2034,7 @@ fn remove_liquidity_with_partially_and_any_filling() { Uint128::new(0), Uint128::new(0), Uint128::new(0), - Uint128::new(5) + Uint128::new(0) ] ); let crust_balance = lp_token.balance(&router.wrap(), owner.clone()).unwrap(); @@ -2144,7 +2362,7 @@ fn update_fee() { assert_eq!(fee.lp_fee_percent, lp_fee_percent); // Try updating with fee values that are too high - let lp_fee_percent = Decimal::from_str("1.01").unwrap(); + let lp_fee_percent = Decimal::from_str("5.01").unwrap(); let protocol_fee_percent = Decimal::zero(); let msg = ExecuteMsg::UpdateFee { protocol_fee_recipient: "new_fee_recipient".to_string(), @@ -2156,8 +2374,8 @@ fn update_fee() { .unwrap_err(); assert_eq!( ContractError::FeesTooHigh { - max_fee_percent: Decimal::from_str("1").unwrap(), - total_fee_percent: Decimal::from_str("1.01").unwrap() + max_fee_percent: Decimal::from_str(PREDEFINED_MAX_FEES_PERCENT).unwrap(), + total_fee_percent: Decimal::from_str("5.01").unwrap() }, err.downcast().unwrap() ); diff --git a/ixo-swap/src/msg.rs b/ixo-swap/src/msg.rs index aeb0ef2..3d2673a 100644 --- a/ixo-swap/src/msg.rs +++ b/ixo-swap/src/msg.rs @@ -230,8 +230,8 @@ impl Denom { impl TokenSelect { pub fn to_string(&self) -> String { match self { - TokenSelect::Token1155 => "Token1155".to_string(), - TokenSelect::Token2 => "Token2".to_string(), + TokenSelect::Token1155 => "token1155".to_string(), + TokenSelect::Token2 => "token2".to_string(), } } } \ No newline at end of file diff --git a/ixo-swap/src/token_amount.rs b/ixo-swap/src/token_amount.rs index b618ca5..42d103c 100644 --- a/ixo-swap/src/token_amount.rs +++ b/ixo-swap/src/token_amount.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, convert::TryFrom}; use cosmwasm_schema::cw_serde; -use cosmwasm_std::{CheckedMultiplyFractionError, Decimal, StdError, Uint128}; +use cosmwasm_std::{CheckedMultiplyFractionError, Decimal, Uint128}; use cw1155::TokenId; use crate::{ @@ -16,6 +16,8 @@ pub enum TokenAmount { } impl TokenAmount { + /// Returns clone of the multiple amount of tokens. + /// If the amount is a single amount, returns an error. pub fn get_multiple(&self) -> Result, ContractError> { match self { TokenAmount::Multiple(amounts) => Ok(amounts.clone()), @@ -25,6 +27,8 @@ impl TokenAmount { } } + /// Returns clone of the single amount of tokens. + /// If the amount is a multiple amount, returns an error. pub fn get_single(&self) -> Result { match self { TokenAmount::Single(amount) => Ok(amount.clone()), @@ -34,6 +38,9 @@ impl TokenAmount { } } + /// Returns the total amount of tokens. + /// - If the amount is a single amount, returns the amount. + /// - If the amount is a multiple amount, returns the sum of all amounts. pub fn get_total(&self) -> Uint128 { match self { TokenAmount::Multiple(amounts) => amounts @@ -46,6 +53,12 @@ impl TokenAmount { } } + /// Wrapper function that returns the amount of tokens for the provided percent. + /// This only gets used to calculate the protocol fees and if it is not zero then we always + /// round up for protocol fees to have minimum 1 token if the protocol fees is not zero + /// - If the percent is zero, returns None. + /// - If the amount is a single amount, returns the single amount after running get_percent_from_single + /// - If the amount is a multiple amount, returns the multiple amount after running get_percent_from_multiple pub fn get_percent(&self, percent: Decimal) -> Result, ContractError> { if percent.is_zero() { return Ok(None); @@ -64,62 +77,66 @@ impl TokenAmount { } } + /// Calculates the amount of tokens with the given percent from a multiple amount. Does so by: + /// - getting the total percentage amount wanted, and use this as counter to run loop till it is 0 + /// - sort the input_amounts by amount (ascending), then by id (lexicographical order) + /// - run for loop and add the amount of tokens wanted to the amounts HashMap, till percent_amount_left is zero + /// + /// NOTE: not all the tokens in the input_amounts will be included in the return(fee_amounts), only the ones with the + /// lowest amounts tot total the percent needed fn get_percent_from_multiple( input_amounts: HashMap, percent: Uint128, ) -> Result { let mut amounts: HashMap = HashMap::new(); let input_amounts_total = TokenAmount::Multiple(input_amounts.clone()).get_total(); + + // Total percentage amount used as counter for the loop let mut percent_amount_left = Self::get_percent_from_single(input_amounts_total, percent)?.get_single()?; - while !percent_amount_left.is_zero() { - let percent_amount_per_token = percent_amount_left - .checked_div(Uint128::from(input_amounts.len() as u32)) - .map_err(StdError::divide_by_zero)?; - - for (token_id, token_amount) in input_amounts.clone().into_iter() { - if percent_amount_left.is_zero() { - break; - } - - let mut taken_percent_amount_per_token = - *amounts.get(&token_id).unwrap_or(&Uint128::zero()); - if taken_percent_amount_per_token == token_amount { - continue; - } - - let token_amount_left = token_amount - taken_percent_amount_per_token; - let percent_amount = if percent_amount_per_token.is_zero() { - percent_amount_left - } else { - percent_amount_per_token - }; - - if token_amount_left >= percent_amount { - taken_percent_amount_per_token += percent_amount; - - if percent_amount_per_token.is_zero() { - percent_amount_left = Uint128::zero(); - } else { - percent_amount_left -= percent_amount_per_token; - } - } else { - taken_percent_amount_per_token += token_amount_left; - percent_amount_left -= token_amount_left; - } - - amounts.insert(token_id, taken_percent_amount_per_token); + // Convert HashMap to Vec of tuples for deterministic sorting + let mut sorted_input_amounts: Vec<(String, Uint128)> = input_amounts.clone().into_iter().collect(); + // Sort by amount (ascending), then by id (lexicographical order) + sorted_input_amounts.sort_by(|a, b| { + if a.1 == b.1 { + a.0.cmp(&b.0) // Sort by TokenId if amounts are equal + } else { + a.1.cmp(&b.1) // Sort by amount (ascending order) + } + }); + + for (token_id, token_amount) in sorted_input_amounts.into_iter() { + if percent_amount_left.is_zero() { + break; } + + // Determine the amount to take from the current token + let take_amount = if token_amount >= percent_amount_left { + percent_amount_left + } else { + token_amount + }; + + // Update the amounts HashMap with the determined take amount + amounts.insert(token_id.clone(), take_amount); + + // Reduce the remaining amount to be taken + percent_amount_left -= take_amount; } Ok(TokenAmount::Multiple(amounts)) } + /// Calculates the amount of tokens with the given percent from a single amount. fn get_percent_from_single( input_amount: Uint128, percent: Uint128, ) -> Result { + if percent.is_zero() || input_amount.is_zero() { + return Ok(TokenAmount::Single(Uint128::zero())); + } + let fraction = (SCALE_FACTOR.u128(), 1u128); let result = input_amount .full_mul(percent) @@ -139,11 +156,23 @@ mod tests { fn should_return_fee_amount_when_single_input_token_provided() { let token_amount = TokenAmount::Single(Uint128::new(26000)); let fee = token_amount - .get_percent(Decimal::from_str("1").unwrap()) + .get_percent(Decimal::from_str("0.01").unwrap()) .unwrap() .unwrap(); - assert_eq!(fee.get_total(), Uint128::new(260)) + assert_eq!(fee.get_total(), Uint128::new(3)) + } + + #[test] + fn should_return_min_fee_amount_when_fee_and_input_low() { + let token_amount = TokenAmount::Single(Uint128::new(260)); + let fee = token_amount + .get_percent(Decimal::from_str("0.01").unwrap()) + .unwrap() + .unwrap(); + + // 0.01% of 260 = 0.26 so it should be rounded up to 1 + assert_eq!(fee.get_total(), Uint128::new(1)) } #[test] @@ -155,11 +184,11 @@ mod tests { ("4".to_string(), Uint128::new(8746)), ])); let fee = token_amount - .get_percent(Decimal::from_str("1").unwrap()) + .get_percent(Decimal::from_str("0.1").unwrap()) .unwrap() .unwrap(); - assert_eq!(fee.get_total(), Uint128::new(260)) + assert_eq!(fee.get_total(), Uint128::new(26)) } #[test] diff --git a/ixo-swap/src/utils.rs b/ixo-swap/src/utils.rs index 8c7107b..aa5e961 100644 --- a/ixo-swap/src/utils.rs +++ b/ixo-swap/src/utils.rs @@ -1,8 +1,11 @@ use cosmwasm_std::{Decimal, StdError, StdResult, Uint128}; +/// The minimum fee percent allowed is 0.01%, based of the SCALE_FACTOR, +/// otherwise it will always end up with 0 fee if lower than 0.01% +pub const MIN_FEE_PERCENT: &str = "0.01"; pub const SCALE_FACTOR: Uint128 = Uint128::new(10_000); -pub const PREDEFINED_MAX_PERCENT: &str = "1"; -pub const PREDEFINED_MAX_SLIPPAGE_PERCENT: &str = "0.5"; +pub const PREDEFINED_MAX_FEES_PERCENT: &str = "5"; +pub const PREDEFINED_MAX_SLIPPAGE_PERCENT: &str = "10"; pub const DECIMAL_PRECISION: Uint128 = Uint128::new(10u128.pow(20)); pub fn decimal_to_uint128(decimal: Decimal) -> StdResult { From aa0c30c3b17184c379461f95c3b8e393887c5f39 Mon Sep 17 00:00:00 2001 From: Michael Pretorius Date: Wed, 4 Sep 2024 21:34:04 +0200 Subject: [PATCH 3/3] fix: fix max_slippage to be same as fee with SCALE_FACTOR --- ixo-swap/src/contract.rs | 18 ++++++------ ixo-swap/src/integration_test.rs | 50 ++++++++++++++++---------------- ixo-swap/src/token_amount.rs | 27 +++++++++-------- ixo-swap/src/utils.rs | 22 +++++++++++++- 4 files changed, 70 insertions(+), 47 deletions(-) diff --git a/ixo-swap/src/contract.rs b/ixo-swap/src/contract.rs index 2135d07..e37d195 100644 --- a/ixo-swap/src/contract.rs +++ b/ixo-swap/src/contract.rs @@ -27,7 +27,7 @@ use crate::state::{ }; use crate::token_amount::TokenAmount; use crate::utils::{ - decimal_to_uint128, MIN_FEE_PERCENT, PREDEFINED_MAX_FEES_PERCENT, PREDEFINED_MAX_SLIPPAGE_PERCENT, SCALE_FACTOR + calculate_amount_with_percent, decimal_to_uint128, MIN_FEE_PERCENT, PREDEFINED_MAX_FEES_PERCENT, PREDEFINED_MAX_SLIPPAGE_PERCENT, SCALE_FACTOR }; // Version info for migration info @@ -580,12 +580,12 @@ fn validate_slippage( min_token_amount: Uint128, actual_token_amount: Uint128, ) -> Result<(), ContractError> { - let max_slippage_percent = MAX_SLIPPAGE_PERCENT.load(deps.storage)?; + let max_slippage = MAX_SLIPPAGE_PERCENT.load(deps.storage)?; + let max_slippage_percent = decimal_to_uint128(max_slippage)?; + + let slippage_impact = calculate_amount_with_percent(actual_token_amount, max_slippage_percent)?; - let actual_token_decimal_amount = Decimal::from_str(actual_token_amount.to_string().as_str())?; - let slippage_impact = actual_token_decimal_amount * max_slippage_percent; - let min_required_decimal_amount = actual_token_decimal_amount - slippage_impact; - let min_required_amount = min_required_decimal_amount.to_uint_floor(); + let min_required_amount = actual_token_amount - slippage_impact; if min_token_amount < min_required_amount { return Err(ContractError::MinTokenAmountError { @@ -2179,7 +2179,7 @@ mod tests { let mut deps = mock_dependencies(); MAX_SLIPPAGE_PERCENT - .save(&mut deps.storage, &Decimal::from_str("0.1").unwrap()) + .save(&mut deps.storage, &Decimal::from_str("2").unwrap()) .unwrap(); let min_token_amount = Uint128::new(95_000); @@ -2191,7 +2191,7 @@ mod tests { err, ContractError::MinTokenAmountError { min_token: min_token_amount, - min_required: Uint128::new(99_000) + min_required: Uint128::new(107_800) // 110_000 * 0.98 (2% slippage) = 107_800 } ); } @@ -2204,7 +2204,7 @@ mod tests { .save(&mut deps.storage, &Decimal::from_str("0.1").unwrap()) .unwrap(); - let min_token_amount = Uint128::new(108_000); + let min_token_amount = Uint128::new(109_900); // 110_000 * 0.999 (0.1% slippage) = 109_890 let actual_token_amount = Uint128::new(110_000); let res = validate_slippage(&deps.as_mut(), min_token_amount, actual_token_amount).unwrap(); diff --git a/ixo-swap/src/integration_test.rs b/ixo-swap/src/integration_test.rs index 94f0069..3dd5616 100644 --- a/ixo-swap/src/integration_test.rs +++ b/ixo-swap/src/integration_test.rs @@ -775,7 +775,7 @@ fn cw1155_to_cw20_swap() { let token_ids = vec![TokenId::from("FIRST/1"), TokenId::from("FIRST/2")]; - let max_slippage_percent = Decimal::from_str("0.3").unwrap(); + let max_slippage_percent = Decimal::from_str("8").unwrap(); let lp_fee_percent = Decimal::from_str("0.2").unwrap(); let protocol_fee_percent = Decimal::from_str("0.1").unwrap(); @@ -899,7 +899,6 @@ fn cw1155_to_cw20_swap() { let res = router .execute_contract(owner.clone(), amm.clone(), &swap_msg, &[]) .unwrap(); - println!("res: {:?}", res); let event = Event::new("wasm").add_attributes(vec![ attr("action", "swap"), attr("sender", owner.clone()), @@ -938,8 +937,8 @@ fn cw1155_to_cw20_swap() { input_token: TokenSelect::Token2, input_amount: TokenAmount::Single(Uint128::new(60_000)), min_output: TokenAmount::Multiple(HashMap::from([ - (token_ids[0].clone(), Uint128::new(30_000)), - (token_ids[1].clone(), Uint128::new(30_000)), + (token_ids[0].clone(), Uint128::new(33_000)), + (token_ids[1].clone(), Uint128::new(33_000)), ])), expiration: None, }; @@ -950,7 +949,7 @@ fn cw1155_to_cw20_swap() { // ensure balances updated let owner_balance = batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; - assert_eq!(owner_balance, [Uint128::new(65_878), Uint128::new(55_000)]); + assert_eq!(owner_balance, [Uint128::new(62_878), Uint128::new(58_000)]); let owner_balance = cw20_token.balance(&router.wrap(), owner.clone()).unwrap(); assert_eq!(owner_balance, Uint128::new(23_266)); let fee_recipient_balance = cw20_token @@ -984,7 +983,7 @@ fn cw1155_to_native_swap() { let cw1155_token = create_cw1155(&mut router, &owner); let token_ids = vec![TokenId::from("FIRST/1"), TokenId::from("FIRST/2")]; - let max_slippage_percent = Decimal::from_str("0.3").unwrap(); + let max_slippage_percent = Decimal::from_str("8").unwrap(); let lp_fee_percent = Decimal::from_str("0.2").unwrap(); let protocol_fee_percent = Decimal::from_str("0.1").unwrap(); @@ -1087,8 +1086,8 @@ fn cw1155_to_native_swap() { input_token: TokenSelect::Token2, input_amount: TokenAmount::Single(Uint128::new(60_000)), min_output: TokenAmount::Multiple(HashMap::from([ - (token_ids[0].clone(), Uint128::new(30_000)), - (token_ids[1].clone(), Uint128::new(30_000)), + (token_ids[0].clone(), Uint128::new(34_000)), + (token_ids[1].clone(), Uint128::new(34_000)), ])), expiration: None, }; @@ -1107,7 +1106,7 @@ fn cw1155_to_native_swap() { // ensure balances updated let owner_balance = batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; - assert_eq!(owner_balance, [Uint128::new(65_878), Uint128::new(55_000)]); + assert_eq!(owner_balance, [Uint128::new(61_878), Uint128::new(59_000)]); let owner_balance: Coin = bank_balance(&mut router, &owner, NATIVE_TOKEN_DENOM.to_string()); assert_eq!(owner_balance.amount, Uint128::new(23_266)); let fee_recipient_balance = bank_balance( @@ -1143,7 +1142,7 @@ fn cw1155_to_native_swap_low_fees() { let cw1155_token = create_cw1155(&mut router, &owner); let token_ids = vec![TokenId::from("FIRST/1"), TokenId::from("FIRST/2")]; - let max_slippage_percent = Decimal::from_str("0.3").unwrap(); + let max_slippage_percent = Decimal::from_str("8").unwrap(); let lp_fee_percent = Decimal::from_str("0.0").unwrap(); let protocol_fee_percent = Decimal::from_str("0.01").unwrap(); @@ -1223,7 +1222,7 @@ fn cw1155_to_native_swap_low_fees() { (token_ids[0].clone(), Uint128::new(5_000)), (token_ids[1].clone(), Uint128::new(5_000)), ])), - min_output: TokenAmount::Single(Uint128::new(6_500)), + min_output: TokenAmount::Single(Uint128::new(8_362)), expiration: None, }; let _res = router @@ -1247,8 +1246,8 @@ fn cw1155_to_native_swap_low_fees() { input_token: TokenSelect::Token2, input_amount: TokenAmount::Single(Uint128::new(7_000)), min_output: TokenAmount::Multiple(HashMap::from([ - (token_ids[0].clone(), Uint128::new(3_000)), - (token_ids[1].clone(), Uint128::new(3_000)), + (token_ids[0].clone(), Uint128::new(3_700)), + (token_ids[1].clone(), Uint128::new(3_700)), ])), expiration: None, }; @@ -1267,7 +1266,7 @@ fn cw1155_to_native_swap_low_fees() { // ensure balances updated let owner_balance = batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; - assert_eq!(owner_balance, [Uint128::new(49_863), Uint128::new(48_000)]); + assert_eq!(owner_balance, [Uint128::new(49_163), Uint128::new(48_700)]); let owner_balance: Coin = bank_balance(&mut router, &owner, NATIVE_TOKEN_DENOM.to_string()); assert_eq!(owner_balance.amount, Uint128::new(52_090)); let fee_recipient_balance = bank_balance( @@ -1336,7 +1335,7 @@ fn amm_add_and_remove_liquidity() { let cw1155_token = create_cw1155(&mut router, &owner); - let max_slippage_percent = Decimal::from_str("0.3").unwrap(); + let max_slippage_percent = Decimal::from_str("1").unwrap(); let supported_denom = "CARBON".to_string(); let token_ids = vec![ @@ -1718,7 +1717,7 @@ fn amm_add_and_remove_liquidity() { let remove_liquidity_msg = ExecuteMsg::RemoveLiquidity { amount: Uint128::new(50), min_token1155: TokenAmount::Multiple(HashMap::from([ - (token_ids[0].clone(), Uint128::new(35)), + (token_ids[0].clone(), Uint128::new(45)), (token_ids[1].clone(), Uint128::new(5)), ])), min_token2: Uint128::new(50), @@ -1741,12 +1740,12 @@ fn amm_add_and_remove_liquidity() { // ensure balances updated let owner_balance = batch_balance_for_owner(&router, &cw1155_token, &owner, &token_ids).balances; - assert_eq!(owner_balance, [Uint128::new(4915), Uint128::new(4985)]); + assert_eq!(owner_balance, [Uint128::new(4925), Uint128::new(4975)]); let token_supplies = get_owner_lp_tokens_balance(&router, &amm_addr, &token_ids).supplies; - assert_eq!(token_supplies, [Uint128::new(85), Uint128::new(15)]); + assert_eq!(token_supplies, [Uint128::new(75), Uint128::new(25)]); let amm_balances = batch_balance_for_owner(&router, &cw1155_token, &amm_addr, &token_ids).balances; - assert_eq!(amm_balances, [Uint128::new(85), Uint128::new(15)]); + assert_eq!(amm_balances, [Uint128::new(75), Uint128::new(25)]); let crust_balance = lp_token.balance(&router.wrap(), owner.clone()).unwrap(); assert_eq!(crust_balance, Uint128::new(100)); @@ -1763,8 +1762,8 @@ fn amm_add_and_remove_liquidity() { let remove_liquidity_msg = ExecuteMsg::RemoveLiquidity { amount: Uint128::new(100), min_token1155: TokenAmount::Multiple(HashMap::from([ - (token_ids[0].clone(), Uint128::new(85)), - (token_ids[1].clone(), Uint128::new(15)), + (token_ids[0].clone(), Uint128::new(75)), + (token_ids[1].clone(), Uint128::new(25)), ])), min_token2: Uint128::new(100), expiration: None, @@ -1803,7 +1802,7 @@ fn remove_liquidity_with_partially_and_any_filling() { let cw1155_token = create_cw1155(&mut router, &owner); - let max_slippage_percent = Decimal::from_str("0.3").unwrap(); + let max_slippage_percent = Decimal::from_str("5").unwrap(); let supported_denom = "CARBON".to_string(); let token_ids = vec![ @@ -1938,8 +1937,9 @@ fn remove_liquidity_with_partially_and_any_filling() { let remove_liquidity_msg = ExecuteMsg::RemoveLiquidity { amount: Uint128::new(80), min_token1155: TokenAmount::Multiple(HashMap::from([ - (token_ids[0].clone(), Uint128::new(40)), + (token_ids[0].clone(), Uint128::new(41)), (token_ids[1].clone(), Uint128::new(30)), + (token_ids[2].clone(), Uint128::new(5)), ])), min_token2: Uint128::new(80), expiration: None, @@ -1996,8 +1996,8 @@ fn remove_liquidity_with_partially_and_any_filling() { let remove_liquidity_msg = ExecuteMsg::RemoveLiquidity { amount: Uint128::new(55), - min_token1155: TokenAmount::Single(Uint128::new(40)), - min_token2: Uint128::new(40), + min_token1155: TokenAmount::Single(Uint128::new(52)), + min_token2: Uint128::new(52), expiration: None, }; let _res = router diff --git a/ixo-swap/src/token_amount.rs b/ixo-swap/src/token_amount.rs index 42d103c..28241f8 100644 --- a/ixo-swap/src/token_amount.rs +++ b/ixo-swap/src/token_amount.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, convert::TryFrom}; +use std::collections::HashMap; use cosmwasm_schema::cw_serde; use cosmwasm_std::{CheckedMultiplyFractionError, Decimal, Uint128}; @@ -6,7 +6,7 @@ use cw1155::TokenId; use crate::{ error::ContractError, - utils::{decimal_to_uint128, SCALE_FACTOR}, + utils::{calculate_amount_with_percent, decimal_to_uint128}, }; #[cw_serde] @@ -133,16 +133,8 @@ impl TokenAmount { input_amount: Uint128, percent: Uint128, ) -> Result { - if percent.is_zero() || input_amount.is_zero() { - return Ok(TokenAmount::Single(Uint128::zero())); - } - - let fraction = (SCALE_FACTOR.u128(), 1u128); - let result = input_amount - .full_mul(percent) - .checked_div_ceil(fraction) - .map_err(|err| err)?; - Ok(TokenAmount::Single(Uint128::try_from(result)?)) + let result = calculate_amount_with_percent(input_amount, percent)?; + Ok(TokenAmount::Single(result)) } } @@ -175,6 +167,17 @@ mod tests { assert_eq!(fee.get_total(), Uint128::new(1)) } + #[test] + fn should_return_zero_when_input_is_zero() { + let token_amount = TokenAmount::Single(Uint128::new(0)); + let fee = token_amount + .get_percent(Decimal::from_str("0.1").unwrap()) + .unwrap() + .unwrap(); + + assert_eq!(fee.get_total(), Uint128::new(0)) + } + #[test] fn should_return_fee_amount_when_multiple_input_token_provided_and_two_token_amount_are_over() { let token_amount = TokenAmount::Multiple(HashMap::from([ diff --git a/ixo-swap/src/utils.rs b/ixo-swap/src/utils.rs index aa5e961..13ac2c4 100644 --- a/ixo-swap/src/utils.rs +++ b/ixo-swap/src/utils.rs @@ -1,4 +1,6 @@ -use cosmwasm_std::{Decimal, StdError, StdResult, Uint128}; +use std::convert::TryFrom; + +use cosmwasm_std::{CheckedMultiplyFractionError, Decimal, StdError, StdResult, Uint128}; /// The minimum fee percent allowed is 0.01%, based of the SCALE_FACTOR, /// otherwise it will always end up with 0 fee if lower than 0.01% @@ -8,6 +10,7 @@ pub const PREDEFINED_MAX_FEES_PERCENT: &str = "5"; pub const PREDEFINED_MAX_SLIPPAGE_PERCENT: &str = "10"; pub const DECIMAL_PRECISION: Uint128 = Uint128::new(10u128.pow(20)); +/// Converts a Decimal to a Uint128 with the SCALE_FACTOR applied, so that Uint128::1 is 0.01% pub fn decimal_to_uint128(decimal: Decimal) -> StdResult { let result: Uint128 = decimal .atomics() @@ -16,3 +19,20 @@ pub fn decimal_to_uint128(decimal: Decimal) -> StdResult { Ok(result / DECIMAL_PRECISION) } + +// Utility function to calculate amount based on percent +pub fn calculate_amount_with_percent( + input_amount: Uint128, + percent: Uint128, +) -> Result { + if percent.is_zero() || input_amount.is_zero() { + return Ok(Uint128::zero()); + } + + let fraction = (SCALE_FACTOR.u128(), 1u128); + let result = input_amount + .full_mul(percent) + .checked_div_ceil(fraction) + .map_err(|err| err)?; + Ok(Uint128::try_from(result)?) +} \ No newline at end of file