From 3bb5f9691268127ebeb376f0aad4378bbf6f6e67 Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Tue, 19 Mar 2024 14:15:25 +0200 Subject: [PATCH 01/10] pool, phoenix: refactors rust doc and adds macro for asserting approx eq --- contracts/pool/src/contract.rs | 2 +- packages/phoenix/src/utils.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs index 156076e5c..9f3840a2b 100644 --- a/contracts/pool/src/contract.rs +++ b/contracts/pool/src/contract.rs @@ -845,10 +845,10 @@ fn do_swap( /// This function divides the deposit in such a way that when swapping it for the other token, /// the resulting amounts of tokens maintain the current pool's ratio. +/// * `config` - The configuration of the liquidity pool. /// * `a_pool` - The current amount of Token A in the liquidity pool. /// * `b_pool` - The current amount of Token B in the liquidity pool. /// * `deposit` - The total amount of tokens that the user wants to deposit into the liquidity pool. -/// * `sell_a` - A boolean that indicates whether the deposit is in Token A (if true) or in Token B (if false). /// # Returns /// * A tuple `(final_offer_amount, final_ask_amount)`, where `final_offer_amount` is the amount of deposit tokens /// to be swapped, and `final_ask_amount` is the amount of the other tokens that will be received in return. diff --git a/packages/phoenix/src/utils.rs b/packages/phoenix/src/utils.rs index b48486a05..dabd5ba54 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 { From d73a8477649142bf59334125467ae5975737224e Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Tue, 19 Mar 2024 15:09:34 +0200 Subject: [PATCH 02/10] pool: new way to calculate the pool ratio when user provides just a single token --- contracts/pool/src/contract.rs | 111 +++++++++++++++++---------------- contracts/pool/src/error.rs | 1 + 2 files changed, 57 insertions(+), 55 deletions(-) diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs index 9f3840a2b..1c0bdb488 100644 --- a/contracts/pool/src/contract.rs +++ b/contracts/pool/src/contract.rs @@ -17,10 +17,7 @@ use crate::{ token_contract, }; use decimal::Decimal; -use phoenix::{ - utils::{is_approx_ratio, LiquidityPoolInitInfo}, - validate_bps, validate_int_parameters, -}; +use phoenix::{utils::LiquidityPoolInitInfo, validate_bps, validate_int_parameters}; // Metadata that is added on to the WASM custom section contractmeta!( @@ -295,7 +292,7 @@ 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 @@ -307,7 +304,16 @@ impl LiquidityPoolTrait for LiquidityPool { 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 { + log!( + &env, + "Off by more than rounding error! a: {}, b: {}", + actual_b_from_swap, + b_from_swap + ); + panic_with_error!(env, ContractError::OffByMoreThanRoundingError) + } + (a - a_for_swap, actual_b_from_swap) } // Only token B is provided (None, Some(b)) if b > 0 => { @@ -319,7 +325,7 @@ 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 @@ -331,7 +337,16 @@ impl LiquidityPoolTrait for LiquidityPool { 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 { + log!( + &env, + "Off by more than rounding error! a: {}, b: {}", + actual_a_from_swap, + a_from_swap + ); + panic_with_error!(env, ContractError::OffByMoreThanRoundingError) + } + (actual_a_from_swap, b - b_for_swap) } // None or invalid amounts are provided _ => { @@ -878,57 +893,43 @@ fn split_deposit_based_on_pool_ratio( ); } - // 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; - - // Tolerance is the smallest difference in deposit that we care about - let tolerance = 500; + // get pool fee rate + let fee = config.protocol_fee_rate(); + // determine which pool is offer and which is ask + 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 + // formula to calculate final_offer_amount + 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 + // formula to calculate final_ask_amount + 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/error.rs b/contracts/pool/src/error.rs index 5d22d37e1..d82fefd29 100644 --- a/contracts/pool/src/error.rs +++ b/contracts/pool/src/error.rs @@ -29,4 +29,5 @@ pub enum ContractError { SlippageInvalid = 20, SwapMinReceivedBiggerThanReturn = 21, + OffByMoreThanRoundingError = 21, } From 91c84302491be8ac6e3b1b05df6a25fdb25127b6 Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Tue, 19 Mar 2024 15:41:39 +0200 Subject: [PATCH 03/10] pool: adds a test for the split deposit --- contracts/pool/src/tests/swap.rs | 71 +++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/contracts/pool/src/tests/swap.rs b/contracts/pool/src/tests/swap.rs index 8ca77819b..fac649e2f 100644 --- a/contracts/pool/src/tests/swap.rs +++ b/contracts/pool/src/tests/swap.rs @@ -1,4 +1,5 @@ extern crate std; +use phoenix::assert_approx_eq; use pretty_assertions::assert_eq; use soroban_sdk::{ symbol_short, @@ -8,7 +9,10 @@ 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; #[test] @@ -1032,3 +1036,68 @@ fn test_should_fail_when_invalid_ask_asset_min_amount() { let spread = 100i64; // 1% maximum spread allowed pool.swap(&user, &token1.address, &1, &Some(10), &Some(spread)); } + +#[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, + Address::generate(&env), + Address::generate(&env), + ); + + 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_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); +} From 9bab9867817b4119a01e40c82a613d7a4d277033 Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Tue, 19 Mar 2024 17:52:10 +0200 Subject: [PATCH 04/10] pool: first try on using the fee in calculating the split deposit ratio --- contracts/pool/src/contract.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs index 1c0bdb488..ad423e623 100644 --- a/contracts/pool/src/contract.rs +++ b/contracts/pool/src/contract.rs @@ -1,3 +1,4 @@ +use soroban_sdk::testutils::arbitrary::std::dbg; use soroban_sdk::{ contract, contractimpl, contractmeta, log, panic_with_error, Address, BytesN, Env, IntoVal, String, @@ -304,10 +305,15 @@ impl LiquidityPoolTrait for LiquidityPool { None, ); // return: rest of Token A amount, simulated result of swap of portion A + dbg!( + actual_b_from_swap, + b_from_swap, + actual_b_from_swap - b_from_swap + ); if (actual_b_from_swap - b_from_swap).abs() > 1 { log!( &env, - "Off by more than rounding error! a: {}, b: {}", + "Pool: ProvideLiquidity: Off by more than rounding error! a: {}, b: {}", actual_b_from_swap, b_from_swap ); @@ -337,10 +343,15 @@ impl LiquidityPoolTrait for LiquidityPool { None, ); // return: simulated result of swap of portion B, rest of Token B amount + dbg!( + actual_a_from_swap, + a_from_swap, + actual_a_from_swap - a_from_swap + ); if (actual_a_from_swap - a_from_swap).abs() > 1 { log!( &env, - "Off by more than rounding error! a: {}, b: {}", + "Pool: ProvideLiquidity: Off by more than rounding error! a: {}, b: {}", actual_a_from_swap, a_from_swap ); @@ -914,9 +925,12 @@ fn split_deposit_based_on_pool_ratio( }; // formula to calculate final_ask_amount + // we need ti handle the fee here as well let final_ask_amount = { - let numerator = ask_pool * final_offer_amount; + dbg!(ask_pool, final_offer_amount, fee); + let numerator = (ask_pool - ask_pool * fee) * final_offer_amount; let denominator = offer_pool + final_offer_amount; + dbg!(numerator, denominator, numerator / denominator); numerator / denominator }; From 33221812b42de2d339dda068822fac1aa25900c0 Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Wed, 20 Mar 2024 13:53:53 +0200 Subject: [PATCH 05/10] pool: change in numerator logic --- contracts/pool/src/contract.rs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs index ad423e623..5d8c0552d 100644 --- a/contracts/pool/src/contract.rs +++ b/contracts/pool/src/contract.rs @@ -1,4 +1,3 @@ -use soroban_sdk::testutils::arbitrary::std::dbg; use soroban_sdk::{ contract, contractimpl, contractmeta, log, panic_with_error, Address, BytesN, Env, IntoVal, String, @@ -305,11 +304,6 @@ impl LiquidityPoolTrait for LiquidityPool { None, ); // return: rest of Token A amount, simulated result of swap of portion A - dbg!( - actual_b_from_swap, - b_from_swap, - actual_b_from_swap - b_from_swap - ); if (actual_b_from_swap - b_from_swap).abs() > 1 { log!( &env, @@ -343,11 +337,6 @@ impl LiquidityPoolTrait for LiquidityPool { None, ); // return: simulated result of swap of portion B, rest of Token B amount - dbg!( - actual_a_from_swap, - a_from_swap, - actual_a_from_swap - a_from_swap - ); if (actual_a_from_swap - a_from_swap).abs() > 1 { log!( &env, @@ -925,12 +914,11 @@ fn split_deposit_based_on_pool_ratio( }; // formula to calculate final_ask_amount - // we need ti handle the fee here as well + // we need to handle the fee here as well + // we don't change ask_pool offer_pool values based on the fee prior to this method let final_ask_amount = { - dbg!(ask_pool, final_offer_amount, fee); - let numerator = (ask_pool - ask_pool * fee) * final_offer_amount; + let numerator = ask_pool * final_offer_amount; let denominator = offer_pool + final_offer_amount; - dbg!(numerator, denominator, numerator / denominator); numerator / denominator }; From 2c70780690b84c8d3b3282a21fe5f896ca68e519 Mon Sep 17 00:00:00 2001 From: --global <--global> Date: Thu, 28 Mar 2024 00:19:37 +0100 Subject: [PATCH 06/10] Pool: Update log to better represent the actual variables --- contracts/pool/src/contract.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs index 5d8c0552d..31c9ea1c8 100644 --- a/contracts/pool/src/contract.rs +++ b/contracts/pool/src/contract.rs @@ -307,7 +307,7 @@ impl LiquidityPoolTrait for LiquidityPool { if (actual_b_from_swap - b_from_swap).abs() > 1 { log!( &env, - "Pool: ProvideLiquidity: Off by more than rounding error! a: {}, b: {}", + "Pool: ProvideLiquidity: B token off by more than rounding error! actual: {}, predicted: {}", actual_b_from_swap, b_from_swap ); @@ -340,7 +340,7 @@ impl LiquidityPoolTrait for LiquidityPool { if (actual_a_from_swap - a_from_swap).abs() > 1 { log!( &env, - "Pool: ProvideLiquidity: Off by more than rounding error! a: {}, b: {}", + "Pool: ProvideLiquidity: A token off by more than rounding error! actual: {}, predicted: {}", actual_a_from_swap, a_from_swap ); From ea35c67e79c1828a55202ce845fa791d0d73fc13 Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Mon, 22 Apr 2024 12:50:07 +0300 Subject: [PATCH 07/10] pool: after linting --- contracts/pool/src/error.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/pool/src/error.rs b/contracts/pool/src/error.rs index d82fefd29..c4beb5189 100644 --- a/contracts/pool/src/error.rs +++ b/contracts/pool/src/error.rs @@ -27,7 +27,6 @@ pub enum ContractError { TokenABiggerThanTokenB = 18, InvalidBps = 19, SlippageInvalid = 20, - SwapMinReceivedBiggerThanReturn = 21, - OffByMoreThanRoundingError = 21, + OffByMoreThanRoundingError = 22, } From 98702a37223e9f98fec95cbeeef2a70b339fe072 Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Mon, 22 Apr 2024 15:54:41 +0300 Subject: [PATCH 08/10] pool: replaces a test with test_case --- contracts/pool/src/contract.rs | 5 +++++ contracts/pool/src/tests/swap.rs | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs index 31c9ea1c8..17864345f 100644 --- a/contracts/pool/src/contract.rs +++ b/contracts/pool/src/contract.rs @@ -325,6 +325,7 @@ impl LiquidityPoolTrait for LiquidityPool { b, &config.token_b, ); + soroban_sdk::testutils::arbitrary::std::dbg!(b_for_swap, a_from_swap); let actual_a_from_swap = do_swap( env.clone(), sender.clone(), @@ -760,6 +761,10 @@ fn do_swap( ); if let Some(ask_asset_min_amount) = ask_asset_min_amount { + soroban_sdk::testutils::arbitrary::std::dbg!( + ask_asset_min_amount, + compute_swap.return_amount + ); if ask_asset_min_amount > compute_swap.return_amount { log!( &env, diff --git a/contracts/pool/src/tests/swap.rs b/contracts/pool/src/tests/swap.rs index fac649e2f..450bfb5e1 100644 --- a/contracts/pool/src/tests/swap.rs +++ b/contracts/pool/src/tests/swap.rs @@ -1037,8 +1037,9 @@ fn test_should_fail_when_invalid_ask_asset_min_amount() { pool.swap(&user, &token1.address, &1, &Some(10), &Some(spread)); } -#[test] -fn provide_liqudity_single_asset_no_fees_poc_split_good_target() { +#[test_case(0; "when fee is 0%")] +#[test_case(1_000; "when fee is 10%")] +fn provide_liqudity_single_asset_poc_split_good_target(swap_fees: i64) { let env = Env::default(); env.mock_all_auths(); env.budget().reset_unlimited(); @@ -1055,7 +1056,6 @@ fn provide_liqudity_single_asset_no_fees_poc_split_good_target() { let user1 = Address::generate(&env); let user2 = Address::generate(&env); - let swap_fees = 0i64; let pool = deploy_liquidity_pool_contract( &env, None, From dcf4e0d87c73d09cc0e572c5b719cd166eae4789 Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Mon, 22 Apr 2024 17:04:03 +0300 Subject: [PATCH 09/10] pool: wip failing tests because of fee issues --- contracts/pool/src/contract.rs | 5 ----- contracts/pool/src/tests/liquidity.rs | 3 ++- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/contracts/pool/src/contract.rs b/contracts/pool/src/contract.rs index 17864345f..31c9ea1c8 100644 --- a/contracts/pool/src/contract.rs +++ b/contracts/pool/src/contract.rs @@ -325,7 +325,6 @@ impl LiquidityPoolTrait for LiquidityPool { b, &config.token_b, ); - soroban_sdk::testutils::arbitrary::std::dbg!(b_for_swap, a_from_swap); let actual_a_from_swap = do_swap( env.clone(), sender.clone(), @@ -761,10 +760,6 @@ fn do_swap( ); if let Some(ask_asset_min_amount) = ask_asset_min_amount { - soroban_sdk::testutils::arbitrary::std::dbg!( - ask_asset_min_amount, - compute_swap.return_amount - ); if ask_asset_min_amount > compute_swap.return_amount { log!( &env, diff --git a/contracts/pool/src/tests/liquidity.rs b/contracts/pool/src/tests/liquidity.rs index 27ff0b1cd..27b68316a 100644 --- a/contracts/pool/src/tests/liquidity.rs +++ b/contracts/pool/src/tests/liquidity.rs @@ -380,7 +380,7 @@ fn provide_liqudity_single_asset_equal_with_fees() { let stake_manager = Address::generate(&env); let stake_owner = Address::generate(&env); - let swap_fees = 1_000i64; // 10% bps + let swap_fees = 1; // 10% bps let pool = deploy_liquidity_pool_contract( &env, None, @@ -562,6 +562,7 @@ fn provide_liqudity_single_asset_one_third_with_fees() { token2.mint(&user1, &100_000); // providing liquidity with a single asset - token2 pool.provide_liquidity(&user1, &None, &None, &Some(100_000), &None, &None); + soroban_sdk::testutils::arbitrary::std::dbg!("after2"); // before swap : A(10_000_000), B(30_000_000) // since pool is 1/3 algorithm will split it around 15794/52734 // swap 47_226k B for A = 17_548 (-10% fee = 15_793) From fbdec77146e5db9f3a8215ee72b6038ad8355fb3 Mon Sep 17 00:00:00 2001 From: Kaloyan Gangov Date: Mon, 22 Apr 2024 17:04:24 +0300 Subject: [PATCH 10/10] pool: wip - lints --- contracts/pool/src/tests/swap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/pool/src/tests/swap.rs b/contracts/pool/src/tests/swap.rs index 450bfb5e1..5393aedd4 100644 --- a/contracts/pool/src/tests/swap.rs +++ b/contracts/pool/src/tests/swap.rs @@ -1037,8 +1037,8 @@ fn test_should_fail_when_invalid_ask_asset_min_amount() { pool.swap(&user, &token1.address, &1, &Some(10), &Some(spread)); } -#[test_case(0; "when fee is 0%")] -#[test_case(1_000; "when fee is 10%")] +#[test_case(0; "when fee is 0 percent")] +#[test_case(1_000; "when fee is 10 percent")] fn provide_liqudity_single_asset_poc_split_good_target(swap_fees: i64) { let env = Env::default(); env.mock_all_auths();