diff --git a/pallets/order-book/src/lib.rs b/pallets/order-book/src/lib.rs index a5cd8bb09e..5a7983c2a5 100644 --- a/pallets/order-book/src/lib.rs +++ b/pallets/order-book/src/lib.rs @@ -54,10 +54,7 @@ pub mod pallet { use frame_system::pallet_prelude::{OriginFor, *}; use orml_traits::asset_registry::{self, Inspect as _}; use scale_info::TypeInfo; - use sp_arithmetic::{ - traits::{BaseArithmetic, CheckedSub}, - Perquintill, - }; + use sp_arithmetic::traits::{BaseArithmetic, CheckedSub}; use sp_runtime::{ traits::{ AtLeast32BitUnsigned, EnsureAdd, EnsureDiv, EnsureFixedPointNumber, EnsureMul, @@ -361,8 +358,6 @@ pub mod pallet { /// Error when unable to calculate the remaining buy amount for a /// partial order. RemainingBuyAmountError, - /// Error when the ratio provided for a partial error is 0. - InvalidPartialOrderRatio, } #[pallet::call] @@ -599,34 +594,33 @@ pub mod pallet { pub fn fill_order_partial( origin: OriginFor, order_id: T::OrderIdNonce, - fulfillment_ratio: Perquintill, + buy_amount: T::Balance, ) -> DispatchResult { let account_id = ensure_signed(origin)?; let order = >::get(order_id)?; ensure!( - !fulfillment_ratio.is_zero(), - Error::::InvalidPartialOrderRatio - ); - - let partial_buy_amount = fulfillment_ratio.mul_floor(order.buy_amount); - let partial_sell_amount = fulfillment_ratio.mul_floor(order.max_sell_amount); - let remaining_buy_amount = order - .buy_amount - .checked_sub(&partial_buy_amount) - .ok_or(Error::::RemainingBuyAmountError)?; - let partial_fulfillment = !remaining_buy_amount.is_zero(); - - ensure!( - partial_buy_amount >= order.min_fulfillment_amount, + buy_amount >= order.min_fulfillment_amount, Error::::InsufficientOrderSize, ); ensure!( - T::TradeableAsset::can_hold(order.asset_in_id, &account_id, partial_buy_amount), + T::TradeableAsset::can_hold(order.asset_in_id, &account_id, buy_amount), Error::::InsufficientAssetFunds, ); + let sell_amount = Self::convert_with_ratio( + order.asset_in_id, + order.asset_out_id, + order.max_sell_rate, + buy_amount, + )?; + let remaining_buy_amount = order + .buy_amount + .checked_sub(&buy_amount) + .ok_or(Error::::RemainingBuyAmountError)?; + let partial_fulfillment = !remaining_buy_amount.is_zero(); + if partial_fulfillment { Self::update_order( order.placing_account.clone(), @@ -639,7 +633,7 @@ pub mod pallet { T::TradeableAsset::release( order.asset_out_id, &order.placing_account, - partial_sell_amount, + sell_amount, false, )?; @@ -650,7 +644,7 @@ pub mod pallet { order.asset_in_id, &account_id, &order.placing_account, - partial_buy_amount, + buy_amount, false, )?; @@ -658,14 +652,14 @@ pub mod pallet { order.asset_out_id, &order.placing_account, &account_id, - partial_sell_amount, + sell_amount, false, )?; T::FulfilledOrderHook::notify_status_change( order_id, Swap { - amount: partial_buy_amount, + amount: buy_amount, currency_in: order.asset_in_id, currency_out: order.asset_out_id, }, @@ -678,7 +672,7 @@ pub mod pallet { partial_fulfillment, currency_in: order.asset_in_id, currency_out: order.asset_out_id, - fulfillment_amount: partial_buy_amount, + fulfillment_amount: buy_amount, sell_rate_limit: order.max_sell_rate, }); diff --git a/pallets/order-book/src/tests.rs b/pallets/order-book/src/tests.rs index 17cc8db119..cc0e491d6a 100644 --- a/pallets/order-book/src/tests.rs +++ b/pallets/order-book/src/tests.rs @@ -10,12 +10,18 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. -use cfg_primitives::conversion::convert_balance_decimals; use cfg_types::tokens::CurrencyId; -use frame_support::{assert_err, assert_noop, assert_ok, dispatch::RawOrigin}; -use orml_traits::asset_registry::Inspect; +use frame_support::{ + assert_err, assert_noop, assert_ok, + dispatch::RawOrigin, + traits::fungibles::{Inspect, MutateHold}, +}; +use orml_tokens::Error::BalanceTooLow; use sp_arithmetic::Perquintill; -use sp_runtime::{traits::Zero, DispatchError, FixedPointNumber, FixedU128}; +use sp_runtime::{ + traits::{BadOrigin, Zero}, + DispatchError, FixedPointNumber, FixedU128, +}; use super::*; use crate::mock::*; @@ -383,9 +389,107 @@ fn fill_order_full_works() { }); } -#[test] -fn fill_order_partial_works() { - for fulfillment_ratio in 1..100 { +mod fill_order_partial { + use super::*; + + #[test] + fn fill_order_partial_works() { + for fulfillment_ratio in 1..100 { + new_test_ext().execute_with(|| { + let buy_amount = 100 * CURRENCY_AUSD_DECIMALS; + let min_fulfillment_amount = 1 * CURRENCY_AUSD_DECIMALS; + let sell_ratio = FixedU128::checked_from_rational(3u32, 2u32).unwrap(); + + assert_ok!(OrderBook::place_order( + ACCOUNT_0, + DEV_AUSD_CURRENCY_ID, + DEV_USDT_CURRENCY_ID, + buy_amount, + sell_ratio, + min_fulfillment_amount, + )); + + let (order_id, order) = get_account_orders(ACCOUNT_0).unwrap()[0]; + + let fulfillment_ratio = Perquintill::from_percent(fulfillment_ratio); + let partial_buy_amount = fulfillment_ratio.mul_floor(buy_amount); + + assert_ok!(OrderBook::fill_order_partial( + RuntimeOrigin::signed(ACCOUNT_1), + order_id, + partial_buy_amount, + )); + + assert_eq!( + AssetPairOrders::::get(DEV_AUSD_CURRENCY_ID, DEV_USDT_CURRENCY_ID), + vec![order_id] + ); + + let expected_sell_amount = OrderBook::convert_with_ratio( + order.asset_in_id, + order.asset_out_id, + order.max_sell_rate, + partial_buy_amount, + ) + .unwrap(); + + let remaining_buy_amount = buy_amount - partial_buy_amount; + + assert_eq!( + System::events()[2].event, + RuntimeEvent::OrmlTokens(orml_tokens::Event::Unreserved { + currency_id: DEV_USDT_CURRENCY_ID, + who: ACCOUNT_0, + amount: expected_sell_amount + }) + ); + assert_eq!( + System::events()[3].event, + RuntimeEvent::OrderBook(Event::OrderUpdated { + order_id, + account: order.placing_account, + buy_amount: remaining_buy_amount, + sell_rate_limit: order.max_sell_rate, + min_fulfillment_amount: order.min_fulfillment_amount, + }) + ); + assert_eq!( + System::events()[4].event, + RuntimeEvent::OrmlTokens(orml_tokens::Event::Transfer { + currency_id: DEV_AUSD_CURRENCY_ID, + to: ACCOUNT_0, + from: ACCOUNT_1, + amount: partial_buy_amount + }) + ); + assert_eq!( + System::events()[5].event, + RuntimeEvent::OrmlTokens(orml_tokens::Event::Transfer { + currency_id: DEV_USDT_CURRENCY_ID, + to: ACCOUNT_1, + from: ACCOUNT_0, + amount: expected_sell_amount + }) + ); + assert_eq!( + System::events()[6].event, + RuntimeEvent::OrderBook(Event::OrderFulfillment { + order_id, + placing_account: order.placing_account, + fulfilling_account: ACCOUNT_1, + partial_fulfillment: true, + fulfillment_amount: partial_buy_amount, + currency_in: order.asset_in_id, + currency_out: order.asset_out_id, + sell_rate_limit: order.max_sell_rate, + }) + ); + }); + } + } + + #[test] + fn fill_order_partial_with_full_amount_works() { new_test_ext().execute_with(|| { let buy_amount = 100 * CURRENCY_AUSD_DECIMALS; let min_fulfillment_amount = 1 * CURRENCY_AUSD_DECIMALS; @@ -402,81 +506,59 @@ fn fill_order_partial_works() { let (order_id, order) = get_account_orders(ACCOUNT_0).unwrap()[0]; - let fulfillment_ratio = Perquintill::from_percent(fulfillment_ratio); - assert_ok!(OrderBook::fill_order_partial( RuntimeOrigin::signed(ACCOUNT_1), order_id, - fulfillment_ratio, + buy_amount, )); assert_eq!( AssetPairOrders::::get(DEV_AUSD_CURRENCY_ID, DEV_USDT_CURRENCY_ID), - vec![order_id] + vec![] ); - let ausd_decimals = RegistryMock::metadata(&DEV_AUSD_CURRENCY_ID) - .unwrap() - .decimals; - let usdt_decimals = RegistryMock::metadata(&DEV_USDT_CURRENCY_ID) - .unwrap() - .decimals; - - let max_sell_amount = convert_balance_decimals( - ausd_decimals, - usdt_decimals, - sell_ratio.checked_mul_int(buy_amount).unwrap(), + let max_sell_amount = OrderBook::convert_with_ratio( + order.asset_in_id, + order.asset_out_id, + order.max_sell_rate, + buy_amount, ) .unwrap(); - let expected_buy_amount = fulfillment_ratio.mul_floor(buy_amount); - let expected_sell_amount = fulfillment_ratio.mul_floor(max_sell_amount); - let remaining_buy_amount = buy_amount - expected_buy_amount; - assert_eq!( System::events()[2].event, RuntimeEvent::OrmlTokens(orml_tokens::Event::Unreserved { currency_id: DEV_USDT_CURRENCY_ID, who: ACCOUNT_0, - amount: expected_sell_amount + amount: max_sell_amount }) ); assert_eq!( System::events()[3].event, - RuntimeEvent::OrderBook(Event::OrderUpdated { - order_id, - account: order.placing_account, - buy_amount: remaining_buy_amount, - sell_rate_limit: order.max_sell_rate, - min_fulfillment_amount: order.min_fulfillment_amount, - }) - ); - assert_eq!( - System::events()[4].event, RuntimeEvent::OrmlTokens(orml_tokens::Event::Transfer { currency_id: DEV_AUSD_CURRENCY_ID, to: ACCOUNT_0, from: ACCOUNT_1, - amount: expected_buy_amount + amount: buy_amount }) ); assert_eq!( - System::events()[5].event, + System::events()[4].event, RuntimeEvent::OrmlTokens(orml_tokens::Event::Transfer { currency_id: DEV_USDT_CURRENCY_ID, to: ACCOUNT_1, from: ACCOUNT_0, - amount: expected_sell_amount + amount: max_sell_amount }) ); assert_eq!( - System::events()[6].event, + System::events()[5].event, RuntimeEvent::OrderBook(Event::OrderFulfillment { order_id, placing_account: order.placing_account, fulfilling_account: ACCOUNT_1, - partial_fulfillment: true, - fulfillment_amount: expected_buy_amount, + partial_fulfillment: false, + fulfillment_amount: buy_amount, currency_in: order.asset_in_id, currency_out: order.asset_out_id, sell_rate_limit: order.max_sell_rate, @@ -484,98 +566,146 @@ fn fill_order_partial_works() { ); }); } -} -#[test] -fn fill_order_partial_100_percent_works() { - new_test_ext().execute_with(|| { - let buy_amount = 100 * CURRENCY_AUSD_DECIMALS; - let min_fulfillment_amount = 1 * CURRENCY_AUSD_DECIMALS; - let sell_ratio = FixedU128::checked_from_rational(3u32, 2u32).unwrap(); + #[test] + fn fill_order_partial_bad_origin() { + new_test_ext().execute_with(|| { + let buy_amount = 100 * CURRENCY_AUSD_DECIMALS; + let min_fulfillment_amount = 10 * CURRENCY_AUSD_DECIMALS; + let sell_ratio = FixedU128::checked_from_rational(3u32, 2u32).unwrap(); - assert_ok!(OrderBook::place_order( - ACCOUNT_0, - DEV_AUSD_CURRENCY_ID, - DEV_USDT_CURRENCY_ID, - buy_amount, - sell_ratio, - min_fulfillment_amount, - )); + assert_ok!(OrderBook::place_order( + ACCOUNT_0, + DEV_AUSD_CURRENCY_ID, + DEV_USDT_CURRENCY_ID, + buy_amount, + sell_ratio, + min_fulfillment_amount, + )); - let (order_id, order) = get_account_orders(ACCOUNT_0).unwrap()[0]; + let (order_id, _) = get_account_orders(ACCOUNT_0).unwrap()[0]; - let fulfillment_ratio = Perquintill::from_percent(100); + assert_noop!( + OrderBook::fill_order_partial( + RawOrigin::None.into(), + order_id, + min_fulfillment_amount, + ), + BadOrigin + ); + }); + } - assert_ok!(OrderBook::fill_order_partial( - RuntimeOrigin::signed(ACCOUNT_1), - order_id, - fulfillment_ratio, - )); + #[test] + fn fill_order_partial_invalid_order() { + new_test_ext().execute_with(|| { + assert_noop!( + OrderBook::fill_order_partial( + RuntimeOrigin::signed(ACCOUNT_1), + 1234, + 10 * CURRENCY_AUSD_DECIMALS, + ), + Error::::OrderNotFound + ); + }); + } - assert_eq!( - AssetPairOrders::::get(DEV_AUSD_CURRENCY_ID, DEV_USDT_CURRENCY_ID), - vec![] - ); + #[test] + fn fill_order_partial_insufficient_order_size() { + new_test_ext().execute_with(|| { + let buy_amount = 100 * CURRENCY_AUSD_DECIMALS; + let min_fulfillment_amount = 10 * CURRENCY_AUSD_DECIMALS; + let sell_ratio = FixedU128::checked_from_rational(3u32, 2u32).unwrap(); - let ausd_decimals = RegistryMock::metadata(&DEV_AUSD_CURRENCY_ID) - .unwrap() - .decimals; - let usdt_decimals = RegistryMock::metadata(&DEV_USDT_CURRENCY_ID) - .unwrap() - .decimals; + assert_ok!(OrderBook::place_order( + ACCOUNT_0, + DEV_AUSD_CURRENCY_ID, + DEV_USDT_CURRENCY_ID, + buy_amount, + sell_ratio, + min_fulfillment_amount, + )); - let max_sell_amount = convert_balance_decimals( - ausd_decimals, - usdt_decimals, - sell_ratio.checked_mul_int(buy_amount).unwrap(), - ) - .unwrap(); + let (order_id, _) = get_account_orders(ACCOUNT_0).unwrap()[0]; - let expected_buy_amount = fulfillment_ratio.mul_floor(buy_amount); - let expected_sell_amount = fulfillment_ratio.mul_floor(max_sell_amount); + assert_noop!( + OrderBook::fill_order_partial( + RuntimeOrigin::signed(ACCOUNT_1), + order_id, + min_fulfillment_amount - 1 * CURRENCY_AUSD_DECIMALS, + ), + Error::::InsufficientOrderSize + ); + }); + } - let _events = System::events(); + #[test] + fn fill_order_partial_insufficient_asset_funds() { + new_test_ext().execute_with(|| { + let buy_amount = 100 * CURRENCY_AUSD_DECIMALS; + let min_fulfillment_amount = 1 * CURRENCY_AUSD_DECIMALS; + let sell_ratio = FixedU128::checked_from_rational(3u32, 2u32).unwrap(); - assert_eq!( - System::events()[2].event, - RuntimeEvent::OrmlTokens(orml_tokens::Event::Unreserved { - currency_id: DEV_USDT_CURRENCY_ID, - who: ACCOUNT_0, - amount: expected_sell_amount - }) - ); - assert_eq!( - System::events()[3].event, - RuntimeEvent::OrmlTokens(orml_tokens::Event::Transfer { - currency_id: DEV_AUSD_CURRENCY_ID, - to: ACCOUNT_0, - from: ACCOUNT_1, - amount: expected_buy_amount - }) - ); - assert_eq!( - System::events()[4].event, - RuntimeEvent::OrmlTokens(orml_tokens::Event::Transfer { - currency_id: DEV_USDT_CURRENCY_ID, - to: ACCOUNT_1, - from: ACCOUNT_0, - amount: expected_sell_amount - }) - ); - assert_eq!( - System::events()[5].event, - RuntimeEvent::OrderBook(Event::OrderFulfillment { - order_id, - placing_account: order.placing_account, - fulfilling_account: ACCOUNT_1, - partial_fulfillment: false, - fulfillment_amount: expected_buy_amount, - currency_in: order.asset_in_id, - currency_out: order.asset_out_id, - sell_rate_limit: order.max_sell_rate, - }) - ); - }); + assert_ok!(OrderBook::place_order( + ACCOUNT_0, + DEV_AUSD_CURRENCY_ID, + DEV_USDT_CURRENCY_ID, + buy_amount, + sell_ratio, + min_fulfillment_amount, + )); + + let (order_id, _) = get_account_orders(ACCOUNT_0).unwrap()[0]; + + let total_balance = OrmlTokens::balance(DEV_AUSD_CURRENCY_ID, &ACCOUNT_1); + assert_ok!(OrmlTokens::hold( + DEV_AUSD_CURRENCY_ID, + &ACCOUNT_1, + total_balance + )); + + // TODO(cdamian): No way of triggering `InsufficientAssetFunds` given that + // `T::TradeableAsset::can_hold` is always returning true? + + assert_noop!( + OrderBook::fill_order_partial( + RuntimeOrigin::signed(ACCOUNT_1), + order_id, + buy_amount, + ), + BalanceTooLow::, + ); + }); + } + + #[test] + fn fill_order_partial_buy_amount_too_big() { + new_test_ext().execute_with(|| { + let buy_amount = 100 * CURRENCY_AUSD_DECIMALS; + let min_fulfillment_amount = 1 * CURRENCY_AUSD_DECIMALS; + let sell_ratio = FixedU128::checked_from_rational(3u32, 2u32).unwrap(); + + assert_ok!(OrderBook::place_order( + ACCOUNT_0, + DEV_AUSD_CURRENCY_ID, + DEV_USDT_CURRENCY_ID, + buy_amount, + sell_ratio, + min_fulfillment_amount, + )); + + let (order_id, _) = get_account_orders(ACCOUNT_0).unwrap()[0]; + + assert_noop!( + OrderBook::fill_order_partial( + RuntimeOrigin::signed(ACCOUNT_1), + order_id, + buy_amount + 1 * CURRENCY_AUSD_DECIMALS, + ), + Error::::RemainingBuyAmountError + ); + }); + } } #[test]