diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs index 1b711a1f3..b35f4ddfe 100644 --- a/contracts/pool/src/contract.rs +++ b/contracts/pool/src/contract.rs @@ -1,23 +1,25 @@ -use phoenix::utils::LiquidityPoolInitInfo; use soroban_sdk::{ contract, contractimpl, contractmeta, log, panic_with_error, Address, BytesN, Env, IntoVal, }; use num_integer::Roots; -use crate::contracterror::ContractError; -use crate::storage::utils::{is_initialized, set_initialized}; -use crate::storage::{ComputeSwap, LiquidityPoolInfo}; use crate::{ + error::ContractError, stake_contract, storage::{ - get_config, save_config, utils, validate_fee_bps, Asset, Config, PairType, PoolResponse, + get_config, save_config, utils, + utils::{is_initialized, set_initialized}, + validate_fee_bps, Asset, ComputeSwap, Config, LiquidityPoolInfo, PairType, PoolResponse, SimulateReverseSwapResponse, SimulateSwapResponse, }, token_contract, }; use decimal::Decimal; -use phoenix::{utils::is_approx_ratio, validate_bps, validate_int_parameters}; +use phoenix::{ + utils::{is_approx_ratio, LiquidityPoolInitInfo}, + validate_bps, validate_int_parameters, +}; // Metadata that is added on to the WASM custom section contractmeta!( @@ -243,7 +245,7 @@ impl LiquidityPoolTrait for LiquidityPool { // Check if custom_slippage_bps is more than max_allowed_slippage if let Some(custom_slippage) = custom_slippage_bps { if custom_slippage > config.max_allowed_slippage_bps { - panic!("Pool: ProvideLiquidity: Custom slippage tolerance is more than max allowed slippage tolerance"); + panic_with_error!(env, ContractError::ProvideLiquiditySlippageToleranceTooHigh); } } @@ -265,6 +267,7 @@ impl LiquidityPoolTrait for LiquidityPool { } // Only token A is provided (Some(a), None) if a > 0 => { + // @audit let (a_for_swap, b_from_swap) = split_deposit_based_on_pool_ratio( &env, &config, @@ -273,21 +276,24 @@ impl LiquidityPoolTrait for LiquidityPool { a, &config.token_a, ); - do_swap( + let actual_b_from_swap = do_swap( env.clone(), sender.clone(), - // FIXM: Disable Referral struct + // FIXME: Disable Referral struct // None, config.clone().token_a, a_for_swap, None, None, ); - // return: rest of Token A amount, simulated result of swap of portion A - (a - a_for_swap, b_from_swap) + if (actual_b_from_swap - b_from_swap).abs() > 1 { + panic!("Off by more than rounding error! a: {}, b: {}", actual_b_from_swap, b_from_swap); + } + (a - a_for_swap, actual_b_from_swap) } // Only token B is provided (None, Some(b)) if b > 0 => { + // @audit let (b_for_swap, a_from_swap) = split_deposit_based_on_pool_ratio( &env, &config, @@ -296,18 +302,20 @@ impl LiquidityPoolTrait for LiquidityPool { b, &config.token_b, ); - do_swap( + let actual_a_from_swap = do_swap( env.clone(), sender.clone(), - // FIXM: Disable Referral struct + // FIXME: Disable Referral struct // None, config.clone().token_b, b_for_swap, None, None, ); - // return: simulated result of swap of portion B, rest of Token B amount - (a_from_swap, b - b_for_swap) + if (actual_a_from_swap - a_from_swap).abs() > 1 { + panic!("Off by more than rounding error!"); + } + (actual_a_from_swap, b - b_for_swap) } // None or invalid amounts are provided _ => { @@ -315,7 +323,10 @@ impl LiquidityPoolTrait for LiquidityPool { &env, "At least one token must be provided and must be bigger then 0!" ); - panic!("Pool: ProvideLiquidity: At least one token must be provided and must be bigger then 0!"); + panic_with_error!( + env, + ContractError::ProvideLiquidityAtLeastOneTokenMustBeBiggerThenZero + ); } }; @@ -426,9 +437,10 @@ impl LiquidityPoolTrait for LiquidityPool { return_amount_a, return_amount_b ); - panic!( - "Pool: WithdrawLiquidity: Minimum amount of token_a or token_b is not satisfied!" - ) + panic_with_error!( + env, + ContractError::WithdrawLiquidityMinimumAmountOfAOrBIsNotSatisfied + ); } // burn shares @@ -779,62 +791,49 @@ fn split_deposit_based_on_pool_ratio( // Validate the inputs if a_pool <= 0 || b_pool <= 0 || deposit <= 0 { log!(env, "Both pools and deposit must be a positive!"); - panic!( - "Pool: split_deposit_based_on_pool_ratio: Both pools and deposit must be a positive!" + panic_with_error!( + env, + ContractError::SplitDepositBothPoolsAndDepositMustBePositive ); } - // Calculate the current ratio in the pool - let target_ratio = Decimal::from_ratio(b_pool, a_pool); - // Define boundaries for binary search algorithm - let mut low = 0; - let mut high = deposit; + if offer_asset != &config.token_a && offer_asset != &config.token_b { + panic!("Pool: split_deposit_based_on_pool_ratio: asset must be asset a or asset b!"); + } - // Tolerance is the smallest difference in deposit that we care about - let tolerance = 500; + let fee = config.protocol_fee_rate(); + let (offer_pool, ask_pool) = if offer_asset == &config.token_a { + (a_pool, b_pool) + } else { + (b_pool, a_pool) + }; - let mut final_offer_amount = deposit; // amount of deposit tokens to be swapped - let mut final_ask_amount = 0; // amount of other tokens to be received + let final_offer_amount = { + let numerator = deposit * fee - 2 * offer_pool + + (deposit * deposit * fee * fee + + 4 * deposit * offer_pool + + 4 * offer_pool * offer_pool) + .sqrt(); + let denominator = 2 * (fee + Decimal::one()); + numerator / denominator + }; - while high - low > tolerance { - let mid = (low + high) / 2; // Calculate middle point + let final_ask_amount = { + let numerator = ask_pool * final_offer_amount; + let denominator = offer_pool + final_offer_amount; + numerator / denominator + }; - // Simulate swap to get amount of other tokens to be received for `mid` amount of deposit tokens - let SimulateSwapResponse { - ask_amount, - spread_amount: _, - commission_amount: _, - total_return: _, - } = LiquidityPool::simulate_swap(env.clone(), offer_asset.clone(), mid); - - // Update final amounts - final_offer_amount = mid; - final_ask_amount = ask_amount; - - // Calculate the ratio that would result from swapping `mid` deposit tokens - let ratio = if offer_asset == &config.token_a { - Decimal::from_ratio(ask_amount, deposit - mid) - } else { - Decimal::from_ratio(deposit - mid, ask_amount) - }; + log!( + &env, + "log", + a_pool, + b_pool, + deposit, + final_offer_amount, + final_ask_amount + ); - // If the resulting ratio is approximately equal (1%) to the target ratio, break the loop - if is_approx_ratio(ratio, target_ratio, Decimal::percent(1)) { - break; - } - // Update boundaries for the next iteration of the binary search - if ratio > target_ratio { - if offer_asset == &config.token_a { - high = mid; - } else { - low = mid; - } - } else if offer_asset == &config.token_a { - low = mid; - } else { - high = mid; - }; - } (final_offer_amount, final_ask_amount) } diff --git a/contracts/pool/src/contracterror.rs b/contracts/pool/src/contracterror.rs deleted file mode 100644 index 195849035..000000000 --- a/contracts/pool/src/contracterror.rs +++ /dev/null @@ -1,8 +0,0 @@ -use soroban_sdk::contracterror; - -#[contracterror] -#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] -#[repr(u32)] -pub enum ContractError { - SpreadExceedsLimit = 1, -} diff --git a/contracts/pool/src/error.rs b/contracts/pool/src/error.rs new file mode 100644 index 000000000..0aefecaec --- /dev/null +++ b/contracts/pool/src/error.rs @@ -0,0 +1,22 @@ +use soroban_sdk::contracterror; + +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] +#[repr(u32)] +pub enum ContractError { + SpreadExceedsLimit = 1, + + ProvideLiquiditySlippageToleranceTooHigh = 2, + ProvideLiquidityAtLeastOneTokenMustBeBiggerThenZero = 3, + + WithdrawLiquidityMinimumAmountOfAOrBIsNotSatisfied = 4, + SplitDepositBothPoolsAndDepositMustBePositive = 5, + ValidateFeeBpsTotalFeesCantBeGreaterThen100 = 6, + + GetDepositAmountsMinABiggerThenDesiredA = 7, + GetDepositAmountsMinBBiggerThenDesiredB = 8, + GetDepositAmountsAmountABiggerThenDesiredA = 9, + GetDepositAmountsAmountALessThenMinA = 10, + GetDepositAmountsAmountBBiggerThenDesiredB = 11, + GetDepositAmountsAmountBLessThenMinB = 12, +} diff --git a/contracts/pool/src/lib.rs b/contracts/pool/src/lib.rs index 44e046f54..ce56f6b80 100644 --- a/contracts/pool/src/lib.rs +++ b/contracts/pool/src/lib.rs @@ -1,6 +1,6 @@ #![no_std] mod contract; -mod contracterror; +mod error; mod storage; pub mod token_contract { diff --git a/contracts/pool/src/storage.rs b/contracts/pool/src/storage.rs index 29c0fad09..96227cefb 100644 --- a/contracts/pool/src/storage.rs +++ b/contracts/pool/src/storage.rs @@ -1,9 +1,9 @@ use soroban_sdk::{ - contracttype, log, symbol_short, xdr::ToXdr, Address, Bytes, BytesN, ConversionError, Env, - Symbol, TryFromVal, Val, + contracttype, log, panic_with_error, symbol_short, xdr::ToXdr, Address, Bytes, BytesN, + ConversionError, Env, Symbol, TryFromVal, Val, }; -use crate::token_contract; +use crate::{error::ContractError, token_contract}; use decimal::Decimal; #[derive(Clone, Copy)] @@ -58,7 +58,10 @@ const MAX_TOTAL_FEE_BPS: i64 = 10_000; pub fn validate_fee_bps(env: &Env, total_fee_bps: i64) -> i64 { if total_fee_bps > MAX_TOTAL_FEE_BPS { log!(env, "Total fees cannot be greater than 100%"); - panic!("Pool: Validate fee bps: total fees cannot be greater than 100%") + panic_with_error!( + env, + ContractError::ValidateFeeBpsTotalFeesCantBeGreaterThen100 + ); } total_fee_bps } @@ -247,12 +250,12 @@ pub mod utils { if let Some(min_a) = min_a { if min_a > desired_a { - panic!("Pool: Get deposit amounts: min_a > desired_a"); + panic_with_error!(env, ContractError::GetDepositAmountsMinABiggerThenDesiredA); } } if let Some(min_b) = min_b { if min_b > desired_b { - panic!("Pool: Get deposit amounts: min_b > desired_b"); + panic_with_error!(env, ContractError::GetDepositAmountsMinABiggerThenDesiredA); } } @@ -269,7 +272,10 @@ pub mod utils { amount_a, desired_a, ); - panic!("Pool: Get deposit amounts: amount_a > desired_a"); + panic_with_error!( + env, + ContractError::GetDepositAmountsAmountABiggerThenDesiredA + ); } }; if let Some(min_a) = min_a { @@ -280,7 +286,7 @@ pub mod utils { amount_a, min_a ); - panic!("Pool: Get deposit amounts: amount_a < min_a"); + panic_with_error!(env, ContractError::GetDepositAmountsAmountALessThenMinA); } } amount_a @@ -299,7 +305,10 @@ pub mod utils { amount_b, desired_b, ); - panic!("Pool: Get deposit amounts: amount_b > desired_b"); + panic_with_error!( + env, + ContractError::GetDepositAmountsAmountBBiggerThenDesiredB + ); } }; if let Some(min_b) = min_b { @@ -310,7 +319,7 @@ pub mod utils { amount_b, min_b ); - panic!("Pool: Get deposit amounts: amount_b < min_b"); + panic_with_error!(env, ContractError::GetDepositAmountsAmountBLessThenMinB); } } amount_b diff --git a/contracts/pool/src/tests/liquidity.rs b/contracts/pool/src/tests/liquidity.rs index d48cbc6d4..3fb6dac9c 100644 --- a/contracts/pool/src/tests/liquidity.rs +++ b/contracts/pool/src/tests/liquidity.rs @@ -390,7 +390,7 @@ fn provide_liqudity_single_asset_equal_with_fees() { &Some(token_a_amount), &Some(50_000), &None, - &Some(49_000), + &None, &None, ); // before swap : A(10_000_000), B(10_000_000) diff --git a/contracts/pool/src/tests/swap.rs b/contracts/pool/src/tests/swap.rs index 209f79b9e..cda2001d2 100644 --- a/contracts/pool/src/tests/swap.rs +++ b/contracts/pool/src/tests/swap.rs @@ -8,8 +8,12 @@ use soroban_sdk::{ use test_case::test_case; use super::setup::{deploy_liquidity_pool_contract, deploy_token_contract}; -use crate::storage::{Asset, PoolResponse, SimulateReverseSwapResponse, SimulateSwapResponse}; +use crate::{ + storage::{Asset, PoolResponse, SimulateReverseSwapResponse, SimulateSwapResponse}, + token_contract, +}; use decimal::Decimal; +use phoenix::assert_approx_eq; #[test] fn simple_swap() { @@ -724,3 +728,66 @@ fn test_swap_fee_variants(swap_fees: i64, commission_fee: i128) { } ); } + +#[test] +fn provide_liqudity_single_asset_no_fees_poc_split_good_target() { + let env = Env::default(); + env.mock_all_auths(); + env.budget().reset_unlimited(); + + let mut admin1 = Address::generate(&env); + let mut admin2 = Address::generate(&env); + + let mut token1 = deploy_token_contract(&env, &admin1); + let mut token2 = deploy_token_contract(&env, &admin2); + if token2.address < token1.address { + std::mem::swap(&mut token1, &mut token2); + std::mem::swap(&mut admin1, &mut admin2); + } + let user1 = Address::generate(&env); + let user2 = Address::generate(&env); + + let swap_fees = 0i64; + let pool = deploy_liquidity_pool_contract( + &env, + None, + (&token1.address, &token2.address), + swap_fees, + None, + None, + None, + ); + + token1.mint(&user1, &10_000_000); + token2.mint(&user1, &10_000_000); + + pool.provide_liquidity( + &user1, + &Some(10_000_000), + &Some(10_000_000), + &Some(10_000_000), + &Some(10_000_000), + &None, + ); + assert_eq!(token1.balance(&pool.address), 10_000_000); + assert_eq!(token2.balance(&pool.address), 10_000_000); + + token1.mint(&user2, &500_000); + + let user2_token_a_balance_before = token1.balance(&user2); + + pool.provide_liquidity(&user2, &Some(500_000), &None, &None, &None, &None); + + let share_token = pool.query_share_token_address(); + + let user2_lp_balance = token_contract::Client::new(&env, &share_token).balance(&user2); + + // @audit User 2 withdraw the liquidity by burning all its lp token balance. + let (_amount_a, amount_b) = pool.withdraw_liquidity(&user2, &user2_lp_balance, &1, &1); + + pool.swap(&user2, &token2.address, &amount_b, &None, &None); + + let user2_token_a_balance_after = token1.balance(&user2); + + assert_approx_eq!(user2_token_a_balance_before, user2_token_a_balance_after, 5); +} diff --git a/packages/phoenix/src/utils.rs b/packages/phoenix/src/utils.rs index 1395f51d2..1621721ba 100644 --- a/packages/phoenix/src/utils.rs +++ b/packages/phoenix/src/utils.rs @@ -38,6 +38,36 @@ pub fn is_approx_ratio(a: Decimal, b: Decimal, tolerance: Decimal) -> bool { diff <= tolerance } +#[macro_export] +macro_rules! assert_approx_eq { + ($a:expr, $b:expr) => {{ + let eps = 1.0e-6; + let (a, b) = (&$a, &$b); + assert!( + (*a - *b).abs() < eps, + "assertion failed: `(left !== right)` \ + (left: `{:?}`, right: `{:?}`, expect diff: `{:?}`, real diff: `{:?}`)", + *a, + *b, + eps, + (*a - *b).abs() + ); + }}; + ($a:expr, $b:expr, $eps:expr) => {{ + let (a, b) = (&$a, &$b); + let eps = $eps; + assert!( + (*a - *b).abs() < eps, + "assertion failed: `(left !== right)` \ + (left: `{:?}`, right: `{:?}`, expect diff: `{:?}`, real diff: `{:?}`)", + *a, + *b, + eps, + (*a - *b).abs() + ); + }}; +} + #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub struct TokenInitInfo {