AMM-CDA-9 Adds soft and hard failure distinction for AMM and order bo…

…ok errors (#1294)

* add event info

* use slice as function parameter

* update test amm amount out of event

* wip

* handle soft and hard failure

* add order book soft failure

* fix clippy

* fix CI

* remove swaps pallet dependency

primitives/src/hybrid_router_api_types.rs

@@ -38,3 +38,26 @@ pub struct OrderbookTrade<AccountId, Balance> {
     pub filled_taker_amount: Balance,
     pub external_fee: ExternalFee<AccountId, Balance>,
 }
+
+pub trait FailSoft {}
+
+#[derive(Debug)]
+pub enum AmmSoftFail {
+    Numerical,
+}
+
+impl FailSoft for AmmSoftFail {}
+
+#[derive(Debug)]
+pub enum OrderbookSoftFail {
+    BelowMinimumBalance,
+    PartialFillNearFullFillNotAllowed,
+}
+
+impl FailSoft for OrderbookSoftFail {}
+
+#[derive(Debug)]
+pub enum ApiError<S: FailSoft> {
+    SoftFailure(S),
+    HardFailure(DispatchError),
+}

primitives/src/traits/hybrid_router_amm_api.rs

@@ -15,12 +15,15 @@
 // You should have received a copy of the GNU General Public License
 // along with Zeitgeist. If not, see <https://www.gnu.org/licenses/>.
 
-use crate::hybrid_router_api_types::AmmTrade;
+use crate::hybrid_router_api_types::{AmmSoftFail, AmmTrade, ApiError};
 use frame_support::dispatch::DispatchError;
 
 /// A type alias for the return struct of AMM buy and sell.
 pub type AmmTradeOf<T> = AmmTrade<<T as HybridRouterAmmApi>::Balance>;
 
+/// A type alias for the error type of the AMM part of the hybrid router.
+pub type ApiErrorOf = ApiError<AmmSoftFail>;
+
 /// Trait for handling the AMM part of the hybrid router.
 pub trait HybridRouterAmmApi {
     type AccountId;
@@ -93,7 +96,7 @@ pub trait HybridRouterAmmApi {
         asset_out: Self::Asset,
         amount_in: Self::Balance,
         min_amount_out: Self::Balance,
-    ) -> Result<AmmTradeOf<Self>, DispatchError>;
+    ) -> Result<AmmTradeOf<Self>, ApiErrorOf>;
 
     /// Calculates the amount a user has to sell to move the price of `asset` to `until`. Returns
     /// zero if the current spot price is below or equal to `until`.
@@ -133,5 +136,5 @@ pub trait HybridRouterAmmApi {
         asset_in: Self::Asset,
         amount_in: Self::Balance,
         min_amount_out: Self::Balance,
-    ) -> Result<AmmTradeOf<Self>, DispatchError>;
+    ) -> Result<AmmTradeOf<Self>, ApiErrorOf>;
 }

primitives/src/traits/hybrid_router_orderbook_api.rs

@@ -15,16 +15,19 @@
 // You should have received a copy of the GNU General Public License
 // along with Zeitgeist. If not, see <https://www.gnu.org/licenses/>.
 
-use frame_support::dispatch::{DispatchError, DispatchResult};
+use frame_support::dispatch::DispatchError;
 
-use crate::hybrid_router_api_types::OrderbookTrade;
+use crate::hybrid_router_api_types::{ApiError, OrderbookSoftFail, OrderbookTrade};
 
 /// A type alias for the return struct of orderbook trades.
 pub type OrderbookTradeOf<T> = OrderbookTrade<
     <T as HybridRouterOrderbookApi>::AccountId,
     <T as HybridRouterOrderbookApi>::Balance,
 >;
 
+/// A type alias for the error type of the orderbook part of the hybrid router.
+pub type ApiErrorOf = ApiError<OrderbookSoftFail>;
+
 /// Trait for handling the order book part of the hybrid router.
 pub trait HybridRouterOrderbookApi {
     type AccountId;
@@ -54,7 +57,7 @@ pub trait HybridRouterOrderbookApi {
         who: Self::AccountId,
         order_id: Self::OrderId,
         maker_partial_fill: Option<Self::Balance>,
-    ) -> Result<OrderbookTradeOf<Self>, DispatchError>;
+    ) -> Result<OrderbookTradeOf<Self>, ApiErrorOf>;
 
     /// Places an order on the order book.
     ///
@@ -73,5 +76,5 @@ pub trait HybridRouterOrderbookApi {
         maker_amount: Self::Balance,
         taker_asset: Self::Asset,
         taker_amount: Self::Balance,
-    ) -> DispatchResult;
+    ) -> Result<(), ApiErrorOf>;
 }

zrml/hybrid-router/src/lib.rs

@@ -53,12 +53,14 @@ mod pallet {
     use orml_traits::MultiCurrency;
     use sp_runtime::{
         traits::{Get, Zero},
-        DispatchResult,
+        DispatchResult, Saturating,
     };
     #[cfg(feature = "runtime-benchmarks")]
     use zeitgeist_primitives::traits::{CompleteSetOperationsApi, DeployPoolApi};
     use zeitgeist_primitives::{
-        hybrid_router_api_types::{AmmTrade, OrderbookTrade},
+        hybrid_router_api_types::{
+            AmmSoftFail, AmmTrade, ApiError, OrderbookSoftFail, OrderbookTrade,
+        },
         math::{
             checked_ops_res::CheckedSubRes,
             fixed::{BaseProvider, FixedDiv, FixedMul, ZeitgeistBase},
@@ -395,24 +397,42 @@ mod pallet {
                 return Ok((amount_in, None));
             }
 
-            let amm_trade_info = match tx_type {
-                TxType::Buy => T::Amm::buy(
-                    who.clone(),
-                    market_id,
-                    asset,
-                    amm_amount_in,
-                    BalanceOf::<T>::zero(),
-                )?,
-                TxType::Sell => T::Amm::sell(
-                    who.clone(),
-                    market_id,
-                    asset,
-                    amm_amount_in,
-                    BalanceOf::<T>::zero(),
-                )?,
-            };
+            let amm_trade_info = Self::handle_amm_trade(
+                tx_type,
+                who.clone(),
+                market_id,
+                asset,
+                amm_amount_in,
+                BalanceOf::<T>::zero(),
+            )?;
+
+            Ok((amount_in.checked_sub_res(&amm_amount_in)?, amm_trade_info))
+        }
 
-            Ok((amount_in.checked_sub_res(&amm_amount_in)?, Some(amm_trade_info)))
+        fn handle_amm_trade(
+            tx_type: TxType,
+            who: AccountIdOf<T>,
+            market_id: MarketIdOf<T>,
+            asset: AssetOf<T>,
+            amount_in: BalanceOf<T>,
+            min_amount_out: BalanceOf<T>,
+        ) -> Result<Option<AmmTradeOf<T>>, DispatchError> {
+            match tx_type {
+                TxType::Buy => {
+                    match T::Amm::buy(who, market_id, asset, amount_in, min_amount_out) {
+                        Ok(amm_trade) => Ok(Some(amm_trade)),
+                        Err(ApiError::SoftFailure(AmmSoftFail::Numerical)) => Ok(None),
+                        Err(ApiError::HardFailure(dispatch_error)) => Err(dispatch_error),
+                    }
+                }
+                TxType::Sell => {
+                    match T::Amm::sell(who, market_id, asset, amount_in, min_amount_out) {
+                        Ok(amm_trade) => Ok(Some(amm_trade)),
+                        Err(ApiError::SoftFailure(AmmSoftFail::Numerical)) => Ok(None),
+                        Err(ApiError::HardFailure(dispatch_error)) => Err(dispatch_error),
+                    }
+                }
+            }
         }
 
         /// Fills the order from the order book if it exists and meets the price conditions.
@@ -495,18 +515,35 @@ mod pallet {
                 let (_taker_fill, maker_fill) =
                     order.taker_and_maker_fill_from_taker_amount(remaining)?;
                 // and the `maker_partial_fill` of `fill_order` is specified in `taker_asset`
-                let order_trade =
-                    T::OrderBook::fill_order(who.clone(), order_id, Some(maker_fill))?;
-                order_trades.push(order_trade);
-                // `maker_fill` is the amount the order owner (maker) wants to receive
-                remaining = remaining.checked_sub_res(&maker_fill)?;
+                let order_trade_opt = Self::handle_fill_order(who.clone(), order_id, maker_fill)?;
+                if let Some(order_trade) = order_trade_opt {
+                    order_trades.push(order_trade);
+                    // `maker_fill` is the amount the order owner (maker) wants to receive
+                    remaining = remaining.checked_sub_res(&maker_fill)?;
+                }
             }
 
             Ok(OrderAmmTradesInfo { remaining, order_trades, amm_trades })
         }
 
+        fn handle_fill_order(
+            who: AccountIdOf<T>,
+            order_id: OrderId,
+            maker_fill: BalanceOf<T>,
+        ) -> Result<Option<OrderTradeOf<T>>, DispatchError> {
+            match T::OrderBook::fill_order(who, order_id, Some(maker_fill)) {
+                Ok(order_trade) => Ok(Some(order_trade)),
+                Err(ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance))
+                | Err(ApiError::SoftFailure(
+                    OrderbookSoftFail::PartialFillNearFullFillNotAllowed,
+                )) => Ok(None),
+                Err(ApiError::HardFailure(dispatch_error)) => Err(dispatch_error),
+            }
+        }
+
         /// Places a limit order if the strategy is `Strategy::LimitOrder`.
         /// If the strategy is `Strategy::ImmediateOrCancel`, an error is returned.
+        /// A bool is returned to indicate if the order was placed successfully.
         ///
         /// # Arguments
         ///
@@ -525,24 +562,27 @@ mod pallet {
             maker_amount: BalanceOf<T>,
             taker_asset: AssetOf<T>,
             taker_amount: BalanceOf<T>,
-        ) -> DispatchResult {
+        ) -> Result<bool, DispatchError> {
             match strategy {
-                Strategy::ImmediateOrCancel => {
-                    return Err(Error::<T>::CancelStrategyApplied.into());
-                }
+                Strategy::ImmediateOrCancel => Err(Error::<T>::CancelStrategyApplied.into()),
                 Strategy::LimitOrder => {
-                    T::OrderBook::place_order(
+                    match T::OrderBook::place_order(
                         who.clone(),
                         market_id,
                         maker_asset,
                         maker_amount,
                         taker_asset,
                         taker_amount,
-                    )?;
+                    ) {
+                        Ok(()) => Ok(true),
+                        Err(ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance))
+                        | Err(ApiError::SoftFailure(
+                            OrderbookSoftFail::PartialFillNearFullFillNotAllowed,
+                        )) => Ok(false),
+                        Err(ApiError::HardFailure(dispatch_error)) => Err(dispatch_error),
+                    }
                 }
             }
-
-            Ok(())
         }
 
         /// Executes a trade by routing the order to the Automated Market Maker (AMM) and the Order Book
@@ -620,6 +660,8 @@ mod pallet {
                 remaining = amm_trade_info.0;
             }
 
+            let mut limit_order_was_placed = false;
+
             if !remaining.is_zero() {
                 let (maker_asset, maker_amount, taker_asset, taker_amount) = match tx_type {
                     TxType::Buy => {
@@ -638,7 +680,7 @@ mod pallet {
                     }
                 };
 
-                Self::maybe_place_limit_order(
+                limit_order_was_placed = Self::maybe_place_limit_order(
                     strategy,
                     &who,
                     market_id,
@@ -658,7 +700,11 @@ mod pallet {
                 market_id,
                 price_limit,
                 asset_in,
-                amount_in,
+                amount_in: if limit_order_was_placed {
+                    amount_in
+                } else {
+                    amount_in.saturating_sub(remaining)
+                },
                 asset_out,
                 amount_out,
                 external_fee_amount,

zrml/neo-swaps/src/lib.rs

@@ -65,7 +65,7 @@ mod pallet {
     };
     use zeitgeist_primitives::{
         constants::{BASE, CENT},
-        hybrid_router_api_types::AmmTrade,
+        hybrid_router_api_types::{AmmSoftFail, AmmTrade, ApiError},
         math::{
             checked_ops_res::{CheckedAddRes, CheckedSubRes},
             fixed::{BaseProvider, FixedDiv, FixedMul, ZeitgeistBase},
@@ -1005,6 +1005,26 @@ mod pallet {
                 external_fee_percentage.mul_floor(ZeitgeistBase::<BalanceOf<T>>::get()?);
             swap_fee.checked_add_res(&external_fee_fractional)
         }
+
+        fn match_failure(error: DispatchError) -> ApiError<AmmSoftFail> {
+            let spot_price_too_low: DispatchError =
+                Error::<T>::NumericalLimits(NumericalLimitsError::SpotPriceTooLow).into();
+            let spot_price_slipped_too_low: DispatchError =
+                Error::<T>::NumericalLimits(NumericalLimitsError::SpotPriceSlippedTooLow).into();
+            let max_amount_exceeded: DispatchError =
+                Error::<T>::NumericalLimits(NumericalLimitsError::MaxAmountExceeded).into();
+            let min_amount_not_met: DispatchError =
+                Error::<T>::NumericalLimits(NumericalLimitsError::MinAmountNotMet).into();
+            if spot_price_too_low == error
+                || spot_price_slipped_too_low == error
+                || max_amount_exceeded == error
+                || min_amount_not_met == error
+            {
+                ApiError::SoftFailure(AmmSoftFail::Numerical)
+            } else {
+                ApiError::HardFailure(error)
+            }
+        }
     }
 
     impl<T: Config> HybridRouterAmmApi for Pallet<T> {
@@ -1047,8 +1067,9 @@ mod pallet {
             asset_out: Self::Asset,
             amount_in: Self::Balance,
             min_amount_out: Self::Balance,
-        ) -> Result<AmmTradeOf<T>, DispatchError> {
+        ) -> Result<AmmTradeOf<T>, ApiError<AmmSoftFail>> {
             Self::do_buy(who, market_id, asset_out, amount_in, min_amount_out)
+                .map_err(Self::match_failure)
         }
 
         fn calculate_sell_amount_until(
@@ -1066,8 +1087,9 @@ mod pallet {
             asset_out: Self::Asset,
             amount_in: Self::Balance,
             min_amount_out: Self::Balance,
-        ) -> Result<AmmTradeOf<T>, DispatchError> {
+        ) -> Result<AmmTradeOf<T>, ApiError<AmmSoftFail>> {
             Self::do_sell(who, market_id, asset_out, amount_in, min_amount_out)
+                .map_err(Self::match_failure)
         }
     }
 }

zrml/orderbook/src/lib.rs

@@ -37,7 +37,7 @@ use orml_traits::{BalanceStatus, MultiCurrency, NamedMultiReservableCurrency};
 pub use pallet::*;
 use sp_runtime::traits::{Get, Zero};
 use zeitgeist_primitives::{
-    hybrid_router_api_types::{ExternalFee, OrderbookTrade},
+    hybrid_router_api_types::{ApiError, ExternalFee, OrderbookSoftFail, OrderbookTrade},
     math::checked_ops_res::{CheckedAddRes, CheckedSubRes},
     orderbook::{Order, OrderId},
     traits::{DistributeFees, HybridRouterOrderbookApi, MarketCommonsPalletApi},
@@ -508,6 +508,21 @@ mod pallet {
         }
     }
 
+    impl<T: Config> Pallet<T> {
+        fn match_failure(error: DispatchError) -> ApiError<OrderbookSoftFail> {
+            let below_minimum_balance: DispatchError = Error::<T>::BelowMinimumBalance.into();
+            let partial_fill_near_full_fill_not_allowed: DispatchError =
+                Error::<T>::PartialFillNearFullFillNotAllowed.into();
+            if error == below_minimum_balance {
+                ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance)
+            } else if error == partial_fill_near_full_fill_not_allowed {
+                ApiError::SoftFailure(OrderbookSoftFail::PartialFillNearFullFillNotAllowed)
+            } else {
+                ApiError::HardFailure(error)
+            }
+        }
+    }
+
     impl<T: Config> HybridRouterOrderbookApi for Pallet<T> {
         type AccountId = AccountIdOf<T>;
         type MarketId = MarketIdOf<T>;
@@ -524,8 +539,8 @@ mod pallet {
             who: Self::AccountId,
             order_id: Self::OrderId,
             maker_partial_fill: Option<Self::Balance>,
-        ) -> Result<OrderbookTradeOf<T>, DispatchError> {
-            Self::do_fill_order(order_id, who, maker_partial_fill)
+        ) -> Result<OrderbookTradeOf<T>, ApiError<OrderbookSoftFail>> {
+            Self::do_fill_order(order_id, who, maker_partial_fill).map_err(Self::match_failure)
         }
 
         fn place_order(
@@ -535,7 +550,7 @@ mod pallet {
             maker_amount: Self::Balance,
             taker_asset: Self::Asset,
             taker_amount: Self::Balance,
-        ) -> Result<(), DispatchError> {
+        ) -> Result<(), ApiError<OrderbookSoftFail>> {
             Self::do_place_order(
                 who,
                 market_id,
@@ -544,6 +559,7 @@ mod pallet {
                 taker_asset,
                 taker_amount,
             )
+            .map_err(Self::match_failure)
         }
     }
 }