From e22a296b341a9e0d954a1c6f6aee985c00b02d78 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Wed, 17 Apr 2024 17:29:28 +0200 Subject: [PATCH 1/3] add tests that fail and shouldn't --- pallets/foreign-investments/src/tests.rs | 82 +++++++++++++++++++++++ pallets/swaps/src/tests.rs | 84 ++++++++++++++++++++++++ 2 files changed, 166 insertions(+) diff --git a/pallets/foreign-investments/src/tests.rs b/pallets/foreign-investments/src/tests.rs index 8659c575b4..90567b81db 100644 --- a/pallets/foreign-investments/src/tests.rs +++ b/pallets/foreign-investments/src/tests.rs @@ -1592,3 +1592,85 @@ mod notifications { }); } } + +mod zero_amount_order { + use super::*; + + #[test] + fn when_increase_with_zero() { + new_test_ext().execute_with(|| { + util::base_configuration(); + + assert_ok!(ForeignInvestment::increase_foreign_investment( + &USER, + INVESTMENT_ID, + 0, + FOREIGN_CURR + )); + + assert_err!( + Swaps::order_id(&USER, (INVESTMENT_ID, Action::Investment)), + pallet_swaps::Error::::OrderNotFound + ); + }); + } + + #[test] + fn when_increase_after_decrease_but_math_precission() { + new_test_ext().execute_with(|| { + const FOREIGN_AMOUNT: Balance = 100; + + util::base_configuration(); + + MockTokenSwaps::mock_convert_by_market(|to, from, amount_from| { + Ok(match (from, to) { + (POOL_CURR, FOREIGN_CURR) => amount_from / 3 + 1, + (FOREIGN_CURR, POOL_CURR) => amount_from * 3, + _ => unreachable!(), + }) + }); + + assert_ok!(ForeignInvestment::increase_foreign_investment( + &USER, + INVESTMENT_ID, + FOREIGN_AMOUNT, + FOREIGN_CURR + )); + + util::fulfill_last_swap(Action::Investment, FOREIGN_AMOUNT); + + assert!(Swaps::order_id(&USER, (INVESTMENT_ID, Action::Investment)).is_err()); + + assert_ok!(ForeignInvestment::decrease_foreign_investment( + &USER, + INVESTMENT_ID, + FOREIGN_AMOUNT, + FOREIGN_CURR + )); + + MockDecreaseInvestHook::mock_notify_status_change(|_, msg| { + assert_eq!( + msg, + ExecutedForeignDecreaseInvest { + amount_decreased: FOREIGN_AMOUNT + 1, + foreign_currency: FOREIGN_CURR, + amount_remaining: FOREIGN_AMOUNT, + } + ); + Ok(()) + }); + + assert_ok!(ForeignInvestment::increase_foreign_investment( + &USER, + INVESTMENT_ID, + FOREIGN_AMOUNT + 1, + FOREIGN_CURR + )); + + assert_err!( + Swaps::order_id(&USER, (INVESTMENT_ID, Action::Investment)), + pallet_swaps::Error::::OrderNotFound + ); + }); + } +} diff --git a/pallets/swaps/src/tests.rs b/pallets/swaps/src/tests.rs index ce7cb60809..560804e3bf 100644 --- a/pallets/swaps/src/tests.rs +++ b/pallets/swaps/src/tests.rs @@ -490,3 +490,87 @@ mod fulfill { }); } } + +mod zero_amount_order { + use super::*; + + #[test] + fn when_apply_over_no_swap() { + new_test_ext().execute_with(|| { + MockTokenSwaps::mock_place_order(|_, _, _, _, _| { + panic!("this mock should not be called"); + }); + + assert_ok!( + >::apply_swap( + &USER, + SWAP_ID, + Swap { + currency_in: CURRENCY_B, + currency_out: CURRENCY_A, + amount_out: AMOUNT, + }, + ), + SwapStatus { + swapped: 0, + pending: 0, + } + ); + + assert_eq!(OrderIdToSwapId::::get(ORDER_ID), None); + assert_eq!(SwapIdToOrderId::::get((USER, SWAP_ID)), None); + }); + } + + #[test] + fn when_apply_over_smaller_inverse_swap_but_math_precission() { + const AMOUNT_A: Balance = 100; + const NEW_ORDER_ID: OrderId = ORDER_ID + 1; + + new_test_ext().execute_with(|| { + MockTokenSwaps::mock_convert_by_market(|to, from, amount_from| match (from, to) { + (CURRENCY_B, CURRENCY_A) => Ok(amount_from * 3), + (CURRENCY_A, CURRENCY_B) => Ok(amount_from / 3 + 1), + _ => unreachable!(), + }); + MockTokenSwaps::mock_get_order_details(|_| { + // Inverse swap + Some(OrderInfo { + swap: Swap { + currency_in: CURRENCY_A, + currency_out: CURRENCY_B, + amount_out: AMOUNT_A / 3, + }, + ratio: OrderRatio::Market, + }) + }); + MockTokenSwaps::mock_cancel_order(|_| Ok(())); + + MockTokenSwaps::mock_place_order(|_, _, _, amount, _| { + assert_eq!(amount, 0); + panic!("this mock should not be called"); + }); + + Swaps::update_id(&USER, SWAP_ID, Some(ORDER_ID)).unwrap(); + + assert_ok!( + >::apply_swap( + &USER, + SWAP_ID, + Swap { + currency_out: CURRENCY_A, + currency_in: CURRENCY_B, + amount_out: AMOUNT_A - 1, + }, + ), + SwapStatus { + swapped: AMOUNT_A / 3, + pending: 0, + } + ); + + assert_eq!(OrderIdToSwapId::::get(ORDER_ID), None); + assert_eq!(SwapIdToOrderId::::get((USER, SWAP_ID)), None); + }); + } +} From c21a1705990c01856198370a36d31377a2a4e1c1 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Wed, 17 Apr 2024 21:23:03 +0200 Subject: [PATCH 2/3] add fix --- pallets/swaps/src/lib.rs | 40 +++++++++++++++++++++++--------------- pallets/swaps/src/tests.rs | 3 +-- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/pallets/swaps/src/lib.rs b/pallets/swaps/src/lib.rs index 1ab5825c3d..fbd52e27a2 100644 --- a/pallets/swaps/src/lib.rs +++ b/pallets/swaps/src/lib.rs @@ -145,20 +145,24 @@ pub mod pallet { ) -> Result<(SwapStatus, Option), DispatchError> { match over_order_id { None => { - let order_id = T::OrderBook::place_order( - who.clone(), - new_swap.currency_in, - new_swap.currency_out, - new_swap.amount_out, - OrderRatio::Market, - )?; + let order_id = if !new_swap.amount_out.is_zero() { + Some(T::OrderBook::place_order( + who.clone(), + new_swap.currency_in, + new_swap.currency_out, + new_swap.amount_out, + OrderRatio::Market, + )?) + } else { + None + }; Ok(( SwapStatus { swapped: T::Balance::zero(), pending: new_swap.amount_out, }, - Some(order_id), + order_id, )) } Some(order_id) => { @@ -228,20 +232,24 @@ pub mod pallet { let amount_to_swap = new_swap.amount_out.ensure_sub(inverse_swap_amount_in)?; - let order_id = T::OrderBook::place_order( - who.clone(), - new_swap.currency_in, - new_swap.currency_out, - amount_to_swap, - OrderRatio::Market, - )?; + let order_id = if !amount_to_swap.is_zero() { + Some(T::OrderBook::place_order( + who.clone(), + new_swap.currency_in, + new_swap.currency_out, + amount_to_swap, + OrderRatio::Market, + )?) + } else { + None + }; Ok(( SwapStatus { swapped: inverse_swap.amount_out, pending: amount_to_swap, }, - Some(order_id), + order_id, )) } } diff --git a/pallets/swaps/src/tests.rs b/pallets/swaps/src/tests.rs index 560804e3bf..d008648459 100644 --- a/pallets/swaps/src/tests.rs +++ b/pallets/swaps/src/tests.rs @@ -508,7 +508,7 @@ mod zero_amount_order { Swap { currency_in: CURRENCY_B, currency_out: CURRENCY_A, - amount_out: AMOUNT, + amount_out: 0, }, ), SwapStatus { @@ -525,7 +525,6 @@ mod zero_amount_order { #[test] fn when_apply_over_smaller_inverse_swap_but_math_precission() { const AMOUNT_A: Balance = 100; - const NEW_ORDER_ID: OrderId = ORDER_ID + 1; new_test_ext().execute_with(|| { MockTokenSwaps::mock_convert_by_market(|to, from, amount_from| match (from, to) { From 00229e41a19e94651dd56317ddb296a49419f819 Mon Sep 17 00:00:00 2001 From: lemunozm Date: Thu, 18 Apr 2024 08:39:59 +0200 Subject: [PATCH 3/3] add extra tests --- pallets/foreign-investments/src/tests.rs | 41 ++++++++++++++++++++++++ pallets/order-book/src/tests.rs | 41 ++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/pallets/foreign-investments/src/tests.rs b/pallets/foreign-investments/src/tests.rs index 90567b81db..715e03f63a 100644 --- a/pallets/foreign-investments/src/tests.rs +++ b/pallets/foreign-investments/src/tests.rs @@ -1673,4 +1673,45 @@ mod zero_amount_order { ); }); } + + #[test] + fn when_increase_fulfill_is_notified() { + new_test_ext().execute_with(|| { + util::base_configuration(); + + assert_ok!(ForeignInvestment::increase_foreign_investment( + &USER, + INVESTMENT_ID, + AMOUNT, + FOREIGN_CURR + )); + + util::fulfill_last_swap(Action::Investment, 0); + }); + } + + #[test] + fn when_decrease_fulfill_is_notified() { + new_test_ext().execute_with(|| { + util::base_configuration(); + + assert_ok!(ForeignInvestment::increase_foreign_investment( + &USER, + INVESTMENT_ID, + AMOUNT, + FOREIGN_CURR + )); + + util::fulfill_last_swap(Action::Investment, AMOUNT); + + assert_ok!(ForeignInvestment::decrease_foreign_investment( + &USER, + INVESTMENT_ID, + AMOUNT, + FOREIGN_CURR + )); + + util::fulfill_last_swap(Action::Investment, 0); + }); + } } diff --git a/pallets/order-book/src/tests.rs b/pallets/order-book/src/tests.rs index cec52dd9a8..10eeb7d57a 100644 --- a/pallets/order-book/src/tests.rs +++ b/pallets/order-book/src/tests.rs @@ -463,6 +463,47 @@ fn correct_order_details() { }); } +#[test] +fn fulfill_zero_amount_order() { + new_test_ext().execute_with(|| { + let order_id = >::place_order( + FROM, + CURRENCY_B, + CURRENCY_A, + 0, + OrderRatio::Custom(DEFAULT_RATIO), + ) + .unwrap(); + + util::expect_notification(order_id, 0, 0, 0); + + assert_ok!(OrderBook::fill_order( + RuntimeOrigin::signed(FROM), + order_id, + 0 + )); + }); +} + +#[test] +fn close_zero_amount_order() { + new_test_ext().execute_with(|| { + let order_id = >::place_order( + FROM, + CURRENCY_B, + CURRENCY_A, + 0, + OrderRatio::Custom(DEFAULT_RATIO), + ) + .unwrap(); + + assert_ok!(OrderBook::cancel_order( + RuntimeOrigin::signed(FROM), + order_id + )); + }); +} + mod market { use super::*;