From 164a70ccef606005e4ac921e4358cf190abd32ca Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Fri, 29 Mar 2024 08:03:30 +0100 Subject: [PATCH 1/9] add event info --- Cargo.lock | 1 + primitives/src/hybrid_router_api_types.rs | 40 +++++ primitives/src/lib.rs | 1 + .../src/traits/hybrid_router_amm_api.rs | 10 +- .../src/traits/hybrid_router_orderbook_api.rs | 12 +- zrml/hybrid-router/Cargo.toml | 2 + zrml/hybrid-router/src/lib.rs | 163 +++++++++++++----- zrml/hybrid-router/src/tests/buy.rs | 43 ++++- zrml/hybrid-router/src/tests/sell.rs | 54 ++++-- zrml/hybrid-router/src/types.rs | 56 +++++- zrml/neo-swaps/src/lib.rs | 28 +-- zrml/orderbook/src/lib.rs | 48 ++++-- zrml/orderbook/src/tests.rs | 17 +- 13 files changed, 382 insertions(+), 93 deletions(-) create mode 100644 primitives/src/hybrid_router_api_types.rs diff --git a/Cargo.lock b/Cargo.lock index ea3695b84..ef242b4dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14622,6 +14622,7 @@ dependencies = [ "zrml-orderbook", "zrml-prediction-markets", "zrml-simple-disputes", + "zrml-swaps", ] [[package]] diff --git a/primitives/src/hybrid_router_api_types.rs b/primitives/src/hybrid_router_api_types.rs new file mode 100644 index 000000000..57af32cc9 --- /dev/null +++ b/primitives/src/hybrid_router_api_types.rs @@ -0,0 +1,40 @@ +// Copyright 2024 Forecasting Technologies LTD. +// +// This file is part of Zeitgeist. +// +// Zeitgeist is free software: you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the +// Free Software Foundation, either version 3 of the License, or (at +// your option) any later version. +// +// Zeitgeist is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zeitgeist. If not, see . + +use frame_support::pallet_prelude::*; +use scale_info::TypeInfo; + +#[derive(Clone, Copy, PartialEq, Eq, Encode, Decode, Debug, TypeInfo)] +pub struct AmmTrade { + pub amount_in: Balance, + pub amount_out: Balance, + pub swap_fee_amount: Balance, + pub external_fee_amount: Balance, +} + +#[derive(Clone, Copy, PartialEq, Eq, Encode, Decode, Debug, TypeInfo)] +pub struct ExternalFee { + pub account: AccountId, + pub amount: Balance, +} + +#[derive(Clone, Copy, PartialEq, Eq, Encode, Decode, Debug, TypeInfo)] +pub struct OrderbookTrade { + pub filled_maker_amount: Balance, + pub filled_taker_amount: Balance, + pub external_fee: ExternalFee, +} diff --git a/primitives/src/lib.rs b/primitives/src/lib.rs index a2197a950..de0033306 100644 --- a/primitives/src/lib.rs +++ b/primitives/src/lib.rs @@ -22,6 +22,7 @@ extern crate alloc; mod asset; pub mod constants; +pub mod hybrid_router_api_types; mod market; pub mod math; mod max_runtime_usize; diff --git a/primitives/src/traits/hybrid_router_amm_api.rs b/primitives/src/traits/hybrid_router_amm_api.rs index fe58cbaa3..0320ffd7f 100644 --- a/primitives/src/traits/hybrid_router_amm_api.rs +++ b/primitives/src/traits/hybrid_router_amm_api.rs @@ -15,7 +15,11 @@ // You should have received a copy of the GNU General Public License // along with Zeitgeist. If not, see . -use frame_support::dispatch::{DispatchError, DispatchResult}; +use crate::hybrid_router_api_types::AmmTrade; +use frame_support::dispatch::DispatchError; + +/// A type alias for the return struct of AMM buy and sell. +pub type AmmTradeOf = AmmTrade<::Balance>; /// Trait for handling the AMM part of the hybrid router. pub trait HybridRouterAmmApi { @@ -89,7 +93,7 @@ pub trait HybridRouterAmmApi { asset_out: Self::Asset, amount_in: Self::Balance, min_amount_out: Self::Balance, - ) -> DispatchResult; + ) -> Result, DispatchError>; /// 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`. @@ -129,5 +133,5 @@ pub trait HybridRouterAmmApi { asset_in: Self::Asset, amount_in: Self::Balance, min_amount_out: Self::Balance, - ) -> DispatchResult; + ) -> Result, DispatchError>; } diff --git a/primitives/src/traits/hybrid_router_orderbook_api.rs b/primitives/src/traits/hybrid_router_orderbook_api.rs index 76d4c12ec..95763aaf0 100644 --- a/primitives/src/traits/hybrid_router_orderbook_api.rs +++ b/primitives/src/traits/hybrid_router_orderbook_api.rs @@ -17,6 +17,14 @@ use frame_support::dispatch::{DispatchError, DispatchResult}; +use crate::hybrid_router_api_types::OrderbookTrade; + +/// A type alias for the return struct of orderbook trades. +pub type OrderbookTradeOf = OrderbookTrade< + ::AccountId, + ::Balance, +>; + /// Trait for handling the order book part of the hybrid router. pub trait HybridRouterOrderbookApi { type AccountId; @@ -41,12 +49,12 @@ pub trait HybridRouterOrderbookApi { /// - `order_id`: The id of the order to fill. /// - `maker_partial_fill`: The amount to fill the order with. /// - /// Returns the filled order amount. + /// Returns the trade information about the filled maker and taker amounts, and the external fee. fn fill_order( who: Self::AccountId, order_id: Self::OrderId, maker_partial_fill: Option, - ) -> DispatchResult; + ) -> Result, DispatchError>; /// Places an order on the order book. /// diff --git a/zrml/hybrid-router/Cargo.toml b/zrml/hybrid-router/Cargo.toml index 18a8a3812..a222f647a 100644 --- a/zrml/hybrid-router/Cargo.toml +++ b/zrml/hybrid-router/Cargo.toml @@ -30,6 +30,7 @@ zrml-neo-swaps = { workspace = true, optional = true } zrml-orderbook = { workspace = true, optional = true } zrml-prediction-markets = { workspace = true, optional = true } zrml-simple-disputes = { workspace = true, optional = true } +zrml-swaps = { workspace = true, optional = true } [dev-dependencies] env_logger = { workspace = true } @@ -57,6 +58,7 @@ mock = [ "zrml-prediction-markets/default", "zrml-prediction-markets/mock", "zrml-simple-disputes/default", + "zrml-swaps/default", "zrml-authorized/default", "zrml-court/default", "zrml-global-disputes/default", diff --git a/zrml/hybrid-router/src/lib.rs b/zrml/hybrid-router/src/lib.rs index e178286a7..d9b3194a1 100644 --- a/zrml/hybrid-router/src/lib.rs +++ b/zrml/hybrid-router/src/lib.rs @@ -34,7 +34,7 @@ pub use pallet::*; #[frame_support::pallet] mod pallet { use crate::{ - types::{Strategy, TxType}, + types::{OrderAmmTradesInfo, Strategy, Trade, TradeEventInfo, TxType}, weights::WeightInfoZeitgeist, }; use alloc::{vec, vec::Vec}; @@ -58,6 +58,7 @@ mod pallet { #[cfg(feature = "runtime-benchmarks")] use zeitgeist_primitives::traits::{CompleteSetOperationsApi, DeployPoolApi}; use zeitgeist_primitives::{ + hybrid_router_api_types::{AmmTrade, OrderbookTrade}, math::{ checked_ops_res::CheckedSubRes, fixed::{BaseProvider, FixedDiv, FixedMul, ZeitgeistBase}, @@ -138,6 +139,8 @@ mod pallet { pub(crate) type MomentOf = <::MarketCommons as MarketCommonsPalletApi>::Moment; pub(crate) type MarketOf = Market, BalanceOf, BlockNumberFor, MomentOf, Asset>>; + pub(crate) type AmmTradeOf = AmmTrade>; + pub(crate) type OrderTradeOf = OrderbookTrade, BalanceOf>; #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] @@ -151,13 +154,30 @@ mod pallet { { /// A trade was executed. HybridRouterExecuted { + /// The type of transaction (Buy or Sell). tx_type: TxType, + /// The account ID of the user performing the trade. who: AccountIdOf, + /// The ID of the market. market_id: MarketIdOf, + /// The maximum price limit for buying or the minimum price limit for selling. price_limit: BalanceOf, + /// The asset provided by the trader. asset_in: AssetOf, + /// The amount of the `asset_in` provided by the trader. + /// It includes swap and external fees. + /// It is an amount before fees are deducted. amount_in: BalanceOf, + /// The asset received by the trader. asset_out: AssetOf, + /// The aggregated amount of the `asset_out` already received + /// by the trader from AMM and orderbook. + /// It is an amount after fees are deducted. + amount_out: BalanceOf, + /// The external fee amount paid in the base asset. + external_fee_amount: BalanceOf, + /// The swap fee amount paid in the base asset. + swap_fee_amount: BalanceOf, }, } @@ -234,7 +254,7 @@ mod pallet { asset, amount_in, max_price, - &orders, + orders, strategy, )?; @@ -290,7 +310,7 @@ mod pallet { asset, amount_in, min_price, - &orders, + orders, strategy, )?; @@ -347,56 +367,52 @@ mod pallet { asset: AssetOf, amount_in: BalanceOf, price_limit: BalanceOf, - ) -> Result, DispatchError> { + ) -> Result<(BalanceOf, Option>), DispatchError> { if !T::Amm::pool_exists(market_id) { - return Ok(amount_in); + return Ok((amount_in, None)); } let spot_price = T::Amm::get_spot_price(market_id, asset)?; - let mut remaining = amount_in; let amm_amount_in = match tx_type { TxType::Buy => { if spot_price >= price_limit { - return Ok(amount_in); + return Ok((amount_in, None)); } T::Amm::calculate_buy_amount_until(market_id, asset, price_limit)? } TxType::Sell => { if spot_price <= price_limit { - return Ok(amount_in); + return Ok((amount_in, None)); } T::Amm::calculate_sell_amount_until(market_id, asset, price_limit)? } }; - let amm_amount_in = amm_amount_in.min(remaining); + let amm_amount_in = amm_amount_in.min(amount_in); - if !amm_amount_in.is_zero() { - match tx_type { - TxType::Buy => { - T::Amm::buy( - who.clone(), - market_id, - asset, - amm_amount_in, - BalanceOf::::zero(), - )?; - } - TxType::Sell => { - T::Amm::sell( - who.clone(), - market_id, - asset, - amm_amount_in, - BalanceOf::::zero(), - )?; - } - } - remaining = remaining.checked_sub_res(&amm_amount_in)?; + if amm_amount_in.is_zero() { + return Ok((amount_in, None)); } - Ok(remaining) + let amm_trade_info = match tx_type { + TxType::Buy => T::Amm::buy( + who.clone(), + market_id, + asset, + amm_amount_in, + BalanceOf::::zero(), + )?, + TxType::Sell => T::Amm::sell( + who.clone(), + market_id, + asset, + amm_amount_in, + BalanceOf::::zero(), + )?, + }; + + Ok((amount_in.checked_sub_res(&amm_amount_in)?, Some(amm_trade_info))) } /// Fills the order from the order book if it exists and meets the price conditions. @@ -422,7 +438,9 @@ mod pallet { base_asset: AssetOf, asset: AssetOf, price_limit: BalanceOf, - ) -> Result, DispatchError> { + ) -> Result, DispatchError> { + let mut amm_trades = Vec::new(); + let mut order_trades = Vec::new(); for &order_id in orders { if remaining.is_zero() { break; @@ -454,7 +472,7 @@ mod pallet { } } - remaining = Self::maybe_fill_from_amm( + let amm_trade_info = Self::maybe_fill_from_amm( tx_type, who, market_id, @@ -463,6 +481,11 @@ mod pallet { order_price, )?; + if let Some(t) = amm_trade_info.1 { + amm_trades.push(t); + } + remaining = amm_trade_info.0; + if remaining.is_zero() { break; } @@ -472,12 +495,14 @@ 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` - T::OrderBook::fill_order(who.clone(), order_id, Some(maker_fill))?; + 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)?; } - Ok(remaining) + Ok(OrderAmmTradesInfo { remaining, order_trades, amm_trades }) } /// Places a limit order if the strategy is `Strategy::LimitOrder`. @@ -544,7 +569,7 @@ mod pallet { asset: AssetOf, amount_in: BalanceOf, price_limit: BalanceOf, - orders: &[OrderId], + orders: Vec, strategy: Strategy, ) -> DispatchResult { ensure!(amount_in > BalanceOf::::zero(), Error::::AmountIsZero); @@ -563,11 +588,12 @@ mod pallet { }; T::AssetManager::ensure_can_withdraw(asset_in, &who, amount_in)?; + let mut amm_trades: Vec> = Vec::new(); let mut remaining = amount_in; - remaining = Self::maybe_fill_orders( + let order_amm_trades_info = Self::maybe_fill_orders( tx_type, - orders, + &orders, remaining, &who, market_id, @@ -576,8 +602,12 @@ mod pallet { price_limit, )?; + remaining = order_amm_trades_info.remaining; + amm_trades.extend(order_amm_trades_info.amm_trades); + let orderbook_trades = order_amm_trades_info.order_trades; + if !remaining.is_zero() { - remaining = Self::maybe_fill_from_amm( + let amm_trade_info = Self::maybe_fill_from_amm( tx_type, &who, market_id, @@ -585,6 +615,9 @@ mod pallet { remaining, price_limit, )?; + + amm_trades.extend(amm_trade_info.1); + remaining = amm_trade_info.0; } if !remaining.is_zero() { @@ -616,6 +649,9 @@ mod pallet { )?; } + let TradeEventInfo { amount_out, external_fee_amount, swap_fee_amount } = + Self::get_event_info(&who, &orderbook_trades, &amm_trades)?; + Self::deposit_event(Event::HybridRouterExecuted { tx_type, who, @@ -624,9 +660,56 @@ mod pallet { asset_in, amount_in, asset_out, + amount_out, + external_fee_amount, + swap_fee_amount, }); Ok(()) } + + fn get_event_info( + who: &AccountIdOf, + orderbook_trades: &[OrderTradeOf], + amm_trades: &[AmmTradeOf], + ) -> Result, DispatchError> { + orderbook_trades + .iter() + .map(|trade| Trade::::Orderbook(trade)) + .chain(amm_trades.iter().map(|trade| Trade::Amm(*trade))) + .try_fold(TradeEventInfo::::new(), |event_info: TradeEventInfo, trade| { + Self::update_event_info(who, trade, event_info) + }) + } + + fn update_event_info( + who: &AccountIdOf, + trade: Trade, + mut event_info: TradeEventInfo, + ) -> Result, DispatchError> { + match trade { + Trade::Orderbook(orderbook_trade) => { + let external_fee_amount = if &orderbook_trade.external_fee.account == who { + orderbook_trade.external_fee.amount + } else { + BalanceOf::::zero() + }; + event_info.add_amount_out_minus_fees(TradeEventInfo:: { + amount_out: orderbook_trade.filled_maker_amount, + external_fee_amount, + swap_fee_amount: BalanceOf::::zero(), + })?; + } + Trade::Amm(amm_trade) => { + event_info.add_amount_out_and_fees(TradeEventInfo:: { + amount_out: amm_trade.amount_out, + external_fee_amount: amm_trade.external_fee_amount, + swap_fee_amount: amm_trade.swap_fee_amount, + })?; + } + } + + Ok(event_info) + } } } diff --git a/zrml/hybrid-router/src/tests/buy.rs b/zrml/hybrid-router/src/tests/buy.rs index 1558764f0..a000c9165 100644 --- a/zrml/hybrid-router/src/tests/buy.rs +++ b/zrml/hybrid-router/src/tests/buy.rs @@ -696,18 +696,46 @@ fn buy_fails_if_balance_too_low() { #[test] fn buy_emits_event() { ExtBuilder::default().build().execute_with(|| { + let liquidity = _10; + let pivot = _1_100; + let spot_prices = vec![_1_2 + pivot, _1_2 - pivot]; + let swap_fee = CENT; let asset_count = 2u16; - let market_id = create_market( - MARKET_CREATOR, + let market_id = create_market_and_deploy_pool( + ALICE, BASE_ASSET, MarketType::Categorical(asset_count), - ScoringRule::AmmCdaHybrid, + liquidity, + spot_prices.clone(), + swap_fee, ); let asset = Asset::CategoricalOutcome(market_id, 0); - let amount_in = 10 * BASE; - let max_price = (BASE / 2).saturated_into::>(); - let orders = vec![]; + let amount_in = _1000 * 100; + + assert_ok!(AssetManager::deposit(BASE_ASSET, &ALICE, amount_in)); + + let max_price = _9_10.saturated_into::>(); + let orders = (0u128..10u128).collect::>(); + let maker_asset = asset; + let maker_amount: BalanceOf = _20.saturated_into(); + let taker_asset = BASE_ASSET; + let taker_amount = _11.saturated_into::>(); + for (i, _) in orders.iter().enumerate() { + let order_creator = i as AccountIdTest; + let surplus = ((i + 1) as u128) * _1_2; + let taker_amount = taker_amount + surplus.saturated_into::>(); + assert_ok!(AssetManager::deposit(maker_asset, &order_creator, maker_amount)); + assert_ok!(OrderBook::place_order( + RuntimeOrigin::signed(order_creator), + market_id, + maker_asset, + maker_amount, + taker_asset, + taker_amount, + )); + } + let strategy = Strategy::LimitOrder; assert_ok!(HybridRouter::buy( RuntimeOrigin::signed(ALICE), @@ -729,6 +757,9 @@ fn buy_emits_event() { asset_in: BASE_ASSET, amount_in, asset_out: asset, + amount_out: 2301256894490, + external_fee_amount: 3423314400, + swap_fee_amount: 2273314407, } .into(), ); diff --git a/zrml/hybrid-router/src/tests/sell.rs b/zrml/hybrid-router/src/tests/sell.rs index b51cba886..8d4464e1c 100644 --- a/zrml/hybrid-router/src/tests/sell.rs +++ b/zrml/hybrid-router/src/tests/sell.rs @@ -73,7 +73,7 @@ fn sell_to_amm_and_then_fill_specified_order() { market_id, asset_in: asset, amount_in: amm_amount_in, - amount_out: 2832089506, + amount_out: 2775447716, swap_fee_amount: 28320895, external_fee_amount: 28320895, } @@ -447,7 +447,7 @@ fn sell_to_amm() { market_id, asset_in: asset, amount_in: 20000000000, - amount_out: 9653703575, + amount_out: 9460629504, swap_fee_amount: 96537036, external_fee_amount: 96537035, } @@ -624,7 +624,7 @@ fn sell_to_amm_only() { market_id, asset_in: asset, amount_in: 20000000000, - amount_out: 9653703575, + amount_out: 9460629504, swap_fee_amount: 96537036, external_fee_amount: 96537035, } @@ -727,21 +727,48 @@ fn sell_fails_if_balance_too_low() { #[test] fn sell_emits_event() { ExtBuilder::default().build().execute_with(|| { + let liquidity = _10; + let pivot = _1_100; + let spot_prices = vec![_1_2 + pivot, _1_2 - pivot]; + let swap_fee = CENT; let asset_count = 2u16; - let market_id = create_market( - MARKET_CREATOR, + let market_id = create_market_and_deploy_pool( + ALICE, BASE_ASSET, MarketType::Categorical(asset_count), - ScoringRule::AmmCdaHybrid, + liquidity, + spot_prices.clone(), + swap_fee, ); let asset = Asset::CategoricalOutcome(market_id, 0); - let amount_in = 10 * BASE; + let amount_in = _1000 * 100; + + let min_price = _1_100.saturated_into::>(); + let orders = (0u128..50u128).collect::>(); + let maker_asset = BASE_ASSET; + let maker_amount: BalanceOf = _9.saturated_into(); + let taker_asset = asset; + let taker_amount = _100.saturated_into::>(); + for (i, _) in orders.iter().enumerate() { + let order_creator = i as AccountIdTest; + let surplus = ((i + 1) as u128) * _1_2; + let taker_amount = taker_amount + surplus.saturated_into::>(); + assert_ok!(AssetManager::deposit(maker_asset, &order_creator, maker_amount)); + assert_ok!(OrderBook::place_order( + RuntimeOrigin::signed(order_creator), + market_id, + maker_asset, + maker_amount, + taker_asset, + taker_amount, + )); + } + + let order_ids = Orders::::iter().map(|(k, _)| k).collect::>(); assert_ok!(Tokens::set_balance(RuntimeOrigin::root(), ALICE, asset, amount_in, 0)); - let max_price = (BASE / 2).saturated_into::>(); - let orders = vec![]; let strategy = Strategy::LimitOrder; assert_ok!(HybridRouter::sell( RuntimeOrigin::signed(ALICE), @@ -749,8 +776,8 @@ fn sell_emits_event() { asset_count, asset, amount_in, - max_price, - orders, + min_price, + order_ids, strategy, )); @@ -759,10 +786,13 @@ fn sell_emits_event() { tx_type: TxType::Sell, who: ALICE, market_id, - price_limit: max_price, + price_limit: min_price, asset_in: asset, amount_in, asset_out: BASE_ASSET, + amount_out: 4551619284973, + external_fee_amount: 45985911066, + swap_fee_amount: 985911072, } .into(), ); diff --git a/zrml/hybrid-router/src/types.rs b/zrml/hybrid-router/src/types.rs index 582ec4f59..03296651e 100644 --- a/zrml/hybrid-router/src/types.rs +++ b/zrml/hybrid-router/src/types.rs @@ -15,7 +15,12 @@ // You should have received a copy of the GNU General Public License // along with Zeitgeist. If not, see . +use crate::{AmmTradeOf, BalanceOf, Config, OrderTradeOf}; +use alloc::vec::Vec; use frame_support::pallet_prelude::*; +use scale_info::TypeInfo; +use sp_runtime::traits::Zero; +use zeitgeist_primitives::math::checked_ops_res::{CheckedAddRes, CheckedSubRes}; /// Represents the strategy used when placing an order in a trading environment. #[derive( @@ -29,7 +34,7 @@ use frame_support::pallet_prelude::*; Decode, RuntimeDebug, MaxEncodedLen, - scale_info::TypeInfo, + TypeInfo, )] pub enum Strategy { /// The trade is rolled back if it cannot be executed fully. @@ -44,3 +49,52 @@ pub enum TxType { Buy, Sell, } + +#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] +pub enum Trade<'a, T: Config> { + Orderbook(&'a OrderTradeOf), + Amm(AmmTradeOf), +} + +#[derive(Clone, Copy, Debug, Decode, Encode, PartialEq, TypeInfo)] +pub struct TradeEventInfo { + pub amount_out: BalanceOf, + pub external_fee_amount: BalanceOf, + pub swap_fee_amount: BalanceOf, +} + +impl TradeEventInfo { + pub fn new() -> Self { + Self { + amount_out: BalanceOf::::zero(), + external_fee_amount: BalanceOf::::zero(), + swap_fee_amount: BalanceOf::::zero(), + } + } + + pub fn add_amount_out_minus_fees(&mut self, additional: Self) -> Result<(), DispatchError> { + self.external_fee_amount = + self.external_fee_amount.checked_add_res(&additional.external_fee_amount)?; + self.swap_fee_amount = self.swap_fee_amount.checked_add_res(&additional.swap_fee_amount)?; + let fees = additional.external_fee_amount.checked_add_res(&additional.swap_fee_amount)?; + let amount_minus_fees = additional.amount_out.checked_sub_res(&fees)?; + self.amount_out = self.amount_out.checked_add_res(&amount_minus_fees)?; + + Ok(()) + } + + pub fn add_amount_out_and_fees(&mut self, additional: Self) -> Result<(), DispatchError> { + self.external_fee_amount = + self.external_fee_amount.checked_add_res(&additional.external_fee_amount)?; + self.swap_fee_amount = self.swap_fee_amount.checked_add_res(&additional.swap_fee_amount)?; + self.amount_out = self.amount_out.checked_add_res(&additional.amount_out)?; + + Ok(()) + } +} + +pub struct OrderAmmTradesInfo { + pub remaining: BalanceOf, + pub order_trades: Vec>, + pub amm_trades: Vec>, +} diff --git a/zrml/neo-swaps/src/lib.rs b/zrml/neo-swaps/src/lib.rs index a26c346f6..66c6f9be1 100644 --- a/zrml/neo-swaps/src/lib.rs +++ b/zrml/neo-swaps/src/lib.rs @@ -65,6 +65,7 @@ mod pallet { }; use zeitgeist_primitives::{ constants::{BASE, CENT}, + hybrid_router_api_types::AmmTrade, math::{ checked_ops_res::{CheckedAddRes, CheckedSubRes}, fixed::{BaseProvider, FixedDiv, FixedMul, ZeitgeistBase}, @@ -100,6 +101,7 @@ mod pallet { <::MarketCommons as MarketCommonsPalletApi>::MarketId; pub(crate) type LiquidityTreeOf = LiquidityTree::MaxLiquidityTreeDepth>; pub(crate) type PoolOf = Pool>; + pub(crate) type AmmTradeOf = AmmTrade>; #[pallet::config] pub trait Config: frame_system::Config { @@ -164,8 +166,7 @@ mod pallet { external_fee_amount: BalanceOf, }, /// Informant sold a position. `amount_out` is the amount of collateral received by `who`, - /// with swap and external fees not yet deducted. The actual amount received is - /// `amount_out - swap_fee_amount - external_fee_amount`. + /// with swap and external fees already deducted. SellExecuted { who: T::AccountId, market_id: MarketIdOf, @@ -324,7 +325,7 @@ mod pallet { let who = ensure_signed(origin)?; let asset_count_real = T::MarketCommons::market(&market_id)?.outcomes(); ensure!(asset_count == asset_count_real, Error::::IncorrectAssetCount); - Self::do_buy(who, market_id, asset_out, amount_in, min_amount_out)?; + let _ = Self::do_buy(who, market_id, asset_out, amount_in, min_amount_out)?; Ok(Some(T::WeightInfo::buy(asset_count as u32)).into()) } @@ -368,7 +369,7 @@ mod pallet { let who = ensure_signed(origin)?; let asset_count_real = T::MarketCommons::market(&market_id)?.outcomes(); ensure!(asset_count == asset_count_real, Error::::IncorrectAssetCount); - Self::do_sell(who, market_id, asset_in, amount_in, min_amount_out)?; + let _ = Self::do_sell(who, market_id, asset_in, amount_in, min_amount_out)?; Ok(Some(T::WeightInfo::sell(asset_count as u32)).into()) } @@ -535,7 +536,7 @@ mod pallet { asset_out: AssetOf, amount_in: BalanceOf, min_amount_out: BalanceOf, - ) -> DispatchResult { + ) -> Result, DispatchError> { ensure!(amount_in != Zero::zero(), Error::::ZeroAmount); let market = T::MarketCommons::market(&market_id)?; ensure!(market.status == MarketStatus::Active, Error::::MarketNotActive); @@ -584,7 +585,7 @@ mod pallet { swap_fee_amount, external_fee_amount, }); - Ok(()) + Ok(AmmTrade { amount_in, amount_out, swap_fee_amount, external_fee_amount }) }) } @@ -595,7 +596,7 @@ mod pallet { asset_in: AssetOf, amount_in: BalanceOf, min_amount_out: BalanceOf, - ) -> DispatchResult { + ) -> Result, DispatchError> { ensure!(amount_in != Zero::zero(), Error::::ZeroAmount); let market = T::MarketCommons::market(&market_id)?; ensure!(market.status == MarketStatus::Active, Error::::MarketNotActive); @@ -654,11 +655,16 @@ mod pallet { market_id, asset_in, amount_in, - amount_out, + amount_out: amount_out_minus_fees, swap_fee_amount, external_fee_amount, }); - Ok(()) + Ok(AmmTrade { + amount_in, + amount_out: amount_out_minus_fees, + swap_fee_amount, + external_fee_amount, + }) }) } @@ -1041,7 +1047,7 @@ mod pallet { asset_out: Self::Asset, amount_in: Self::Balance, min_amount_out: Self::Balance, - ) -> DispatchResult { + ) -> Result, DispatchError> { Self::do_buy(who, market_id, asset_out, amount_in, min_amount_out) } @@ -1060,7 +1066,7 @@ mod pallet { asset_out: Self::Asset, amount_in: Self::Balance, min_amount_out: Self::Balance, - ) -> DispatchResult { + ) -> Result, DispatchError> { Self::do_sell(who, market_id, asset_out, amount_in, min_amount_out) } } diff --git a/zrml/orderbook/src/lib.rs b/zrml/orderbook/src/lib.rs index 8fbc2c25c..3eeedd49f 100644 --- a/zrml/orderbook/src/lib.rs +++ b/zrml/orderbook/src/lib.rs @@ -37,6 +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}, math::checked_ops_res::{CheckedAddRes, CheckedSubRes}, orderbook::{Order, OrderId}, traits::{DistributeFees, HybridRouterOrderbookApi, MarketCommonsPalletApi}, @@ -109,6 +110,8 @@ mod pallet { MomentOf, AssetOf, >; + pub(crate) type OrderbookTradeOf = OrderbookTrade, BalanceOf>; + pub(crate) type ExternalFeeOf = ExternalFee, BalanceOf>; #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] @@ -134,6 +137,7 @@ mod pallet { filled_taker_amount: BalanceOf, unfilled_maker_amount: BalanceOf, unfilled_taker_amount: BalanceOf, + external_fee: ExternalFeeOf, }, OrderPlaced { order_id: OrderId, @@ -206,7 +210,7 @@ mod pallet { ) -> DispatchResult { let taker = ensure_signed(origin)?; - Self::do_fill_order(order_id, taker, maker_partial_fill)?; + let _ = Self::do_fill_order(order_id, taker, maker_partial_fill)?; Ok(()) } @@ -321,16 +325,22 @@ mod pallet { Ok(()) } - /// Charge the external fees from `taker` and return the adjusted maker fill. + /// Charge the external fees in base asset and return the adjusted maker fill. + /// + /// `maker_fill` is the amount that the maker wants to have. + /// `taker_fill` is the amount that the taker wants to have. + /// It does not charge fees from the outcome asset. + /// + /// Returns the adjusted maker fill and the external fee. fn charge_external_fees( order_data: &OrderOf, base_asset: AssetOf, maker_fill: BalanceOf, taker: &AccountIdOf, taker_fill: BalanceOf, - ) -> Result, DispatchError> { - let maker_asset_is_base = order_data.maker_asset == base_asset; - let base_asset_fill = if maker_asset_is_base { + ) -> Result<(BalanceOf, ExternalFeeOf), DispatchError> { + let maker_asset_is_base_asset = order_data.maker_asset == base_asset; + let base_asset_fill = if maker_asset_is_base_asset { taker_fill } else { debug_assert!(order_data.taker_asset == base_asset); @@ -342,14 +352,14 @@ mod pallet { taker, base_asset_fill, ); - if maker_asset_is_base { - // maker_fill is the amount that the maker wants to have (outcome asset from taker) - // do not charge fees from outcome assets, but rather from the base asset - Ok(maker_fill) + if maker_asset_is_base_asset { + Ok((maker_fill, ExternalFeeOf:: { account: taker.clone(), amount: fee_amount })) } else { - // accounting fees from the taker, - // who is responsible to pay the base asset minus fees to the maker - Ok(maker_fill.checked_sub_res(&fee_amount)?) + Ok(( + // maker gets less base asset, so the maker paid the fees + maker_fill.checked_sub_res(&fee_amount)?, + ExternalFeeOf:: { account: order_data.maker.clone(), amount: fee_amount }, + )) } } @@ -357,7 +367,7 @@ mod pallet { order_id: OrderId, taker: AccountIdOf, maker_partial_fill: Option>, - ) -> DispatchResult { + ) -> Result, DispatchError> { let mut order_data = >::get(order_id).ok_or(Error::::OrderDoesNotExist)?; let market = T::MarketCommons::market(&order_data.market_id)?; debug_assert!( @@ -395,8 +405,7 @@ mod pallet { BalanceStatus::Free, )?; - // always charge fees from the base asset and not the outcome asset - let maybe_adjusted_maker_fill = Self::charge_external_fees( + let (maybe_adjusted_maker_fill, external_fee) = Self::charge_external_fees( &order_data, base_asset, maker_fill, @@ -431,9 +440,14 @@ mod pallet { filled_taker_amount: maker_fill, unfilled_maker_amount: order_data.maker_amount, unfilled_taker_amount: order_data.taker_amount, + external_fee: external_fee.clone(), }); - Ok(()) + Ok(OrderbookTrade { + filled_maker_amount: taker_fill, + filled_taker_amount: maker_fill, + external_fee, + }) } fn do_place_order( @@ -510,7 +524,7 @@ mod pallet { who: Self::AccountId, order_id: Self::OrderId, maker_partial_fill: Option, - ) -> DispatchResult { + ) -> Result, DispatchError> { Self::do_fill_order(order_id, who, maker_partial_fill) } diff --git a/zrml/orderbook/src/tests.rs b/zrml/orderbook/src/tests.rs index dc5003c1d..aed88b2ef 100644 --- a/zrml/orderbook/src/tests.rs +++ b/zrml/orderbook/src/tests.rs @@ -25,6 +25,7 @@ use sp_runtime::{Perbill, Perquintill}; use test_case::test_case; use zeitgeist_primitives::{ constants::BASE, + hybrid_router_api_types::ExternalFee, types::{Asset, MarketStatus, MarketType, ScalarPosition, ScoringRule}, }; use zrml_market_commons::{Error as MError, MarketCommonsPalletApi, Markets}; @@ -570,6 +571,8 @@ fn it_fills_order_fully_maker_outcome_asset() { let reserved_bob = AssetManager::reserved_balance(maker_asset, &BOB); assert_eq!(reserved_bob, 0); + let external_fee = ExternalFee { account: BOB, amount: taker_fees }; + System::assert_last_event( Event::::OrderFilled { order_id, @@ -579,6 +582,7 @@ fn it_fills_order_fully_maker_outcome_asset() { filled_taker_amount: taker_amount, unfilled_maker_amount: 0, unfilled_taker_amount: 0, + external_fee, } .into(), ); @@ -627,6 +631,9 @@ fn it_fills_order_fully_maker_base_asset() { let reserved_bob = AssetManager::reserved_balance(taker_asset, &BOB); assert_eq!(reserved_bob, 0); + let external_fee_amount = calculate_fee::(maker_amount); + let external_fee = ExternalFee { account: BOB, amount: external_fee_amount }; + System::assert_last_event( Event::::OrderFilled { order_id, @@ -636,6 +643,7 @@ fn it_fills_order_fully_maker_base_asset() { filled_taker_amount: taker_amount, unfilled_taker_amount: 0, unfilled_maker_amount: 0, + external_fee, } .into(), ); @@ -699,6 +707,7 @@ fn it_fills_order_partially_maker_base_asset() { let filled_maker_amount = Perquintill::from_rational(alice_portion, taker_amount).mul_floor(maker_amount); let unfilled_maker_amount = maker_amount - filled_maker_amount; + let external_fee_amount = calculate_fee::(filled_maker_amount); assert_eq!( order, @@ -712,6 +721,8 @@ fn it_fills_order_partially_maker_base_asset() { } ); + let external_fee = ExternalFee { account: BOB, amount: external_fee_amount }; + System::assert_last_event( Event::::OrderFilled { order_id, @@ -721,6 +732,7 @@ fn it_fills_order_partially_maker_base_asset() { filled_taker_amount: alice_portion, unfilled_maker_amount, unfilled_taker_amount, + external_fee, } .into(), ); @@ -787,9 +799,10 @@ fn it_fills_order_partially_maker_outcome_asset() { let market_creator_free_balance_after = AssetManager::free_balance(market.base_asset, &MARKET_CREATOR); + let external_fee_amount = calculate_fee::(70 * BASE); assert_eq!( market_creator_free_balance_after - market_creator_free_balance_before, - calculate_fee::(70 * BASE) + external_fee_amount ); let order = Orders::::get(order_id).unwrap(); @@ -810,6 +823,7 @@ fn it_fills_order_partially_maker_outcome_asset() { let reserved_bob = AssetManager::reserved_balance(maker_asset, &BOB); assert_eq!(reserved_bob, filled_maker_amount); + let external_fee = ExternalFee { account: BOB, amount: external_fee_amount }; System::assert_last_event( Event::::OrderFilled { @@ -821,6 +835,7 @@ fn it_fills_order_partially_maker_outcome_asset() { filled_taker_amount: alice_portion, unfilled_maker_amount: filled_maker_amount, unfilled_taker_amount: taker_amount - alice_portion, + external_fee, } .into(), ); From 68db417b1b7621072730952d53f5196a7d4c8e7b Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Fri, 29 Mar 2024 08:03:57 +0100 Subject: [PATCH 2/9] use slice as function parameter --- zrml/hybrid-router/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zrml/hybrid-router/src/lib.rs b/zrml/hybrid-router/src/lib.rs index d9b3194a1..8d97ec532 100644 --- a/zrml/hybrid-router/src/lib.rs +++ b/zrml/hybrid-router/src/lib.rs @@ -254,7 +254,7 @@ mod pallet { asset, amount_in, max_price, - orders, + &orders, strategy, )?; @@ -310,7 +310,7 @@ mod pallet { asset, amount_in, min_price, - orders, + &orders, strategy, )?; @@ -569,7 +569,7 @@ mod pallet { asset: AssetOf, amount_in: BalanceOf, price_limit: BalanceOf, - orders: Vec, + orders: &[OrderId], strategy: Strategy, ) -> DispatchResult { ensure!(amount_in > BalanceOf::::zero(), Error::::AmountIsZero); @@ -593,7 +593,7 @@ mod pallet { let order_amm_trades_info = Self::maybe_fill_orders( tx_type, - &orders, + orders, remaining, &who, market_id, From 0964acccb5f18efb1e265998905bb4e9653d89b4 Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Fri, 29 Mar 2024 08:40:06 +0100 Subject: [PATCH 3/9] update test amm amount out of event --- zrml/neo-swaps/src/tests/sell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zrml/neo-swaps/src/tests/sell.rs b/zrml/neo-swaps/src/tests/sell.rs index 2ac0f38bf..40509917b 100644 --- a/zrml/neo-swaps/src/tests/sell.rs +++ b/zrml/neo-swaps/src/tests/sell.rs @@ -81,7 +81,7 @@ fn sell_works() { market_id, asset_in, amount_in, - amount_out: expected_amount_out, + amount_out: expected_amount_out_minus_fees, swap_fee_amount: expected_swap_fee_amount, external_fee_amount: expected_external_fee_amount, } From 07a5bcef7bc6d16b33370e2acafbe923cf4373aa Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Fri, 29 Mar 2024 10:58:22 +0100 Subject: [PATCH 4/9] wip --- primitives/src/hybrid_router_api_types.rs | 19 ++++++++++++++++++ .../src/traits/hybrid_router_orderbook_api.rs | 11 ++++++---- zrml/hybrid-router/src/lib.rs | 15 ++++++++++++-- zrml/orderbook/src/lib.rs | 20 +++++++++++++++---- 4 files changed, 55 insertions(+), 10 deletions(-) diff --git a/primitives/src/hybrid_router_api_types.rs b/primitives/src/hybrid_router_api_types.rs index 57af32cc9..0cdfcd5ac 100644 --- a/primitives/src/hybrid_router_api_types.rs +++ b/primitives/src/hybrid_router_api_types.rs @@ -38,3 +38,22 @@ pub struct OrderbookTrade { pub filled_taker_amount: Balance, pub external_fee: ExternalFee, } + +pub trait FailSoft {} + +pub enum AmmSoftFail { + Numerical, +} + +impl FailSoft for AmmSoftFail {} + +pub enum OrderbookSoftFail { + BelowMinimumBalance, +} + +impl FailSoft for OrderbookSoftFail {} + +pub enum ApiError { + SoftFailure(S), + HardFailure(DispatchError), +} diff --git a/primitives/src/traits/hybrid_router_orderbook_api.rs b/primitives/src/traits/hybrid_router_orderbook_api.rs index 95763aaf0..cd68710d8 100644 --- a/primitives/src/traits/hybrid_router_orderbook_api.rs +++ b/primitives/src/traits/hybrid_router_orderbook_api.rs @@ -15,9 +15,9 @@ // You should have received a copy of the GNU General Public License // along with Zeitgeist. If not, see . -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 = OrderbookTrade< @@ -25,6 +25,9 @@ pub type OrderbookTradeOf = OrderbookTrade< ::Balance, >; +/// A type alias for the error type of the orderbook part of the hybrid router. +pub type ApiErrorOf = ApiError; + /// 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, - ) -> Result, DispatchError>; + ) -> Result, 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>; } diff --git a/zrml/hybrid-router/src/lib.rs b/zrml/hybrid-router/src/lib.rs index 8d97ec532..557901197 100644 --- a/zrml/hybrid-router/src/lib.rs +++ b/zrml/hybrid-router/src/lib.rs @@ -495,8 +495,7 @@ 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))?; + let order_trade = Self::handle_fill_order(who.clone(), order_id, 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)?; @@ -505,6 +504,18 @@ mod pallet { Ok(OrderAmmTradesInfo { remaining, order_trades, amm_trades }) } + fn handle_fill_order( + who: AccountIdOf, + order_id: OrderId, + maker_fill: BalanceOf, + ) -> Result, DispatchError> { + T::OrderBook::fill_order(who, order_id, Some(maker_fill)).map_err(|err| { + if let Err(DispatchError::Module(ModuleError { index: })) = err { + e + } + }) + } + /// Places a limit order if the strategy is `Strategy::LimitOrder`. /// If the strategy is `Strategy::ImmediateOrCancel`, an error is returned. /// diff --git a/zrml/orderbook/src/lib.rs b/zrml/orderbook/src/lib.rs index 3eeedd49f..ad4917b79 100644 --- a/zrml/orderbook/src/lib.rs +++ b/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,17 @@ mod pallet { } } + impl Pallet { + fn match_failure(error: DispatchError) -> ApiError { + let below_minimum_balance: DispatchError = Error::::BelowMinimumBalance.into(); + if error == below_minimum_balance { + ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance) + } else { + ApiError::HardFailure(error) + } + } + } + impl HybridRouterOrderbookApi for Pallet { type AccountId = AccountIdOf; type MarketId = MarketIdOf; @@ -524,8 +535,8 @@ mod pallet { who: Self::AccountId, order_id: Self::OrderId, maker_partial_fill: Option, - ) -> Result, DispatchError> { - Self::do_fill_order(order_id, who, maker_partial_fill) + ) -> Result, ApiError> { + Self::do_fill_order(order_id, who, maker_partial_fill).map_err(Self::match_failure) } fn place_order( @@ -535,7 +546,7 @@ mod pallet { maker_amount: Self::Balance, taker_asset: Self::Asset, taker_amount: Self::Balance, - ) -> Result<(), DispatchError> { + ) -> Result<(), ApiError> { Self::do_place_order( who, market_id, @@ -544,6 +555,7 @@ mod pallet { taker_asset, taker_amount, ) + .map_err(Self::match_failure) } } } From 50db51d6ba5950306c3c4230f30f96281cbe8430 Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Tue, 2 Apr 2024 14:56:04 +0200 Subject: [PATCH 5/9] handle soft and hard failure --- primitives/src/hybrid_router_api_types.rs | 3 + .../src/traits/hybrid_router_amm_api.rs | 9 +- zrml/hybrid-router/src/lib.rs | 105 ++++++++++++------ zrml/neo-swaps/src/lib.rs | 29 ++++- 4 files changed, 104 insertions(+), 42 deletions(-) diff --git a/primitives/src/hybrid_router_api_types.rs b/primitives/src/hybrid_router_api_types.rs index 0cdfcd5ac..4176462ea 100644 --- a/primitives/src/hybrid_router_api_types.rs +++ b/primitives/src/hybrid_router_api_types.rs @@ -41,18 +41,21 @@ pub struct OrderbookTrade { pub trait FailSoft {} +#[derive(Debug)] pub enum AmmSoftFail { Numerical, } impl FailSoft for AmmSoftFail {} +#[derive(Debug)] pub enum OrderbookSoftFail { BelowMinimumBalance, } impl FailSoft for OrderbookSoftFail {} +#[derive(Debug)] pub enum ApiError { SoftFailure(S), HardFailure(DispatchError), diff --git a/primitives/src/traits/hybrid_router_amm_api.rs b/primitives/src/traits/hybrid_router_amm_api.rs index 0320ffd7f..91489c511 100644 --- a/primitives/src/traits/hybrid_router_amm_api.rs +++ b/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 . -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 = AmmTrade<::Balance>; +/// A type alias for the error type of the AMM part of the hybrid router. +pub type ApiErrorOf = ApiError; + /// 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, DispatchError>; + ) -> Result, 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, DispatchError>; + ) -> Result, ApiErrorOf>; } diff --git a/zrml/hybrid-router/src/lib.rs b/zrml/hybrid-router/src/lib.rs index 557901197..43d4404b2 100644 --- a/zrml/hybrid-router/src/lib.rs +++ b/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::::zero(), - )?, - TxType::Sell => T::Amm::sell( - who.clone(), - market_id, - asset, - amm_amount_in, - BalanceOf::::zero(), - )?, - }; + let amm_trade_info = Self::handle_amm_trade( + tx_type, + who.clone(), + market_id, + asset, + amm_amount_in, + BalanceOf::::zero(), + )?; - Ok((amount_in.checked_sub_res(&amm_amount_in)?, Some(amm_trade_info))) + Ok((amount_in.checked_sub_res(&amm_amount_in)?, amm_trade_info)) + } + + fn handle_amm_trade( + tx_type: TxType, + who: AccountIdOf, + market_id: MarketIdOf, + asset: AssetOf, + amount_in: BalanceOf, + min_amount_out: BalanceOf, + ) -> Result>, 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,10 +515,12 @@ 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 = Self::handle_fill_order(who.clone(), order_id, 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 }) @@ -508,16 +530,17 @@ mod pallet { who: AccountIdOf, order_id: OrderId, maker_fill: BalanceOf, - ) -> Result, DispatchError> { - T::OrderBook::fill_order(who, order_id, Some(maker_fill)).map_err(|err| { - if let Err(DispatchError::Module(ModuleError { index: })) = err { - e - } - }) + ) -> Result>, DispatchError> { + match T::OrderBook::fill_order(who, order_id, Some(maker_fill)) { + Ok(order_trade) => Ok(Some(order_trade)), + Err(ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance)) => 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 /// @@ -536,24 +559,28 @@ mod pallet { maker_amount: BalanceOf, taker_asset: AssetOf, taker_amount: BalanceOf, - ) -> DispatchResult { + ) -> Result { match strategy { Strategy::ImmediateOrCancel => { return Err(Error::::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)) => { + 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 @@ -631,6 +658,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 => { @@ -649,7 +678,7 @@ mod pallet { } }; - Self::maybe_place_limit_order( + limit_order_was_placed = Self::maybe_place_limit_order( strategy, &who, market_id, @@ -669,7 +698,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, diff --git a/zrml/neo-swaps/src/lib.rs b/zrml/neo-swaps/src/lib.rs index 66c6f9be1..58673ec1e 100644 --- a/zrml/neo-swaps/src/lib.rs +++ b/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,27 @@ mod pallet { external_fee_percentage.mul_floor(ZeitgeistBase::>::get()?); swap_fee.checked_add_res(&external_fee_fractional) } + + fn match_failure(error: DispatchError) -> ApiError { + // TODO what else should be a soft failure for the Hybrid Router? + let spot_price_too_low: DispatchError = + Error::::NumericalLimits(NumericalLimitsError::SpotPriceTooLow).into(); + let spot_price_slipped_too_low: DispatchError = + Error::::NumericalLimits(NumericalLimitsError::SpotPriceSlippedTooLow).into(); + let max_amount_exceeded: DispatchError = + Error::::NumericalLimits(NumericalLimitsError::MaxAmountExceeded).into(); + let min_amount_not_met: DispatchError = + Error::::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 HybridRouterAmmApi for Pallet { @@ -1047,8 +1068,9 @@ mod pallet { asset_out: Self::Asset, amount_in: Self::Balance, min_amount_out: Self::Balance, - ) -> Result, DispatchError> { + ) -> Result, ApiError> { 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 +1088,9 @@ mod pallet { asset_out: Self::Asset, amount_in: Self::Balance, min_amount_out: Self::Balance, - ) -> Result, DispatchError> { + ) -> Result, ApiError> { Self::do_sell(who, market_id, asset_out, amount_in, min_amount_out) + .map_err(Self::match_failure) } } } From 43ba4899c857697ba18feefd6df51cd2f7b10275 Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Wed, 3 Apr 2024 11:21:07 +0200 Subject: [PATCH 6/9] add order book soft failure --- primitives/src/hybrid_router_api_types.rs | 1 + zrml/hybrid-router/src/lib.rs | 12 ++++++++---- zrml/neo-swaps/src/lib.rs | 1 - zrml/orderbook/src/lib.rs | 4 ++++ 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/primitives/src/hybrid_router_api_types.rs b/primitives/src/hybrid_router_api_types.rs index 4176462ea..65eb3a50f 100644 --- a/primitives/src/hybrid_router_api_types.rs +++ b/primitives/src/hybrid_router_api_types.rs @@ -51,6 +51,7 @@ impl FailSoft for AmmSoftFail {} #[derive(Debug)] pub enum OrderbookSoftFail { BelowMinimumBalance, + PartialFillNearFullFillNotAllowed, } impl FailSoft for OrderbookSoftFail {} diff --git a/zrml/hybrid-router/src/lib.rs b/zrml/hybrid-router/src/lib.rs index 43d4404b2..91c253527 100644 --- a/zrml/hybrid-router/src/lib.rs +++ b/zrml/hybrid-router/src/lib.rs @@ -533,7 +533,10 @@ mod pallet { ) -> Result>, DispatchError> { match T::OrderBook::fill_order(who, order_id, Some(maker_fill)) { Ok(order_trade) => Ok(Some(order_trade)), - Err(ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance)) => Ok(None), + Err(ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance)) + | Err(ApiError::SoftFailure( + OrderbookSoftFail::PartialFillNearFullFillNotAllowed, + )) => Ok(None), Err(ApiError::HardFailure(dispatch_error)) => Err(dispatch_error), } } @@ -574,9 +577,10 @@ mod pallet { taker_amount, ) { Ok(()) => Ok(true), - Err(ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance)) => { - Ok(false) - } + Err(ApiError::SoftFailure(OrderbookSoftFail::BelowMinimumBalance)) + | Err(ApiError::SoftFailure( + OrderbookSoftFail::PartialFillNearFullFillNotAllowed, + )) => Ok(false), Err(ApiError::HardFailure(dispatch_error)) => Err(dispatch_error), } } diff --git a/zrml/neo-swaps/src/lib.rs b/zrml/neo-swaps/src/lib.rs index 58673ec1e..c8f51fbbe 100644 --- a/zrml/neo-swaps/src/lib.rs +++ b/zrml/neo-swaps/src/lib.rs @@ -1007,7 +1007,6 @@ mod pallet { } fn match_failure(error: DispatchError) -> ApiError { - // TODO what else should be a soft failure for the Hybrid Router? let spot_price_too_low: DispatchError = Error::::NumericalLimits(NumericalLimitsError::SpotPriceTooLow).into(); let spot_price_slipped_too_low: DispatchError = diff --git a/zrml/orderbook/src/lib.rs b/zrml/orderbook/src/lib.rs index ad4917b79..b889ae1c3 100644 --- a/zrml/orderbook/src/lib.rs +++ b/zrml/orderbook/src/lib.rs @@ -511,8 +511,12 @@ mod pallet { impl Pallet { fn match_failure(error: DispatchError) -> ApiError { let below_minimum_balance: DispatchError = Error::::BelowMinimumBalance.into(); + let partial_fill_near_full_fill_not_allowed: DispatchError = + Error::::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) } From e5061b982394e2b64ff856471d355ac0906b78ac Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Wed, 3 Apr 2024 11:25:59 +0200 Subject: [PATCH 7/9] fix clippy --- zrml/hybrid-router/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/zrml/hybrid-router/src/lib.rs b/zrml/hybrid-router/src/lib.rs index 91c253527..16d891922 100644 --- a/zrml/hybrid-router/src/lib.rs +++ b/zrml/hybrid-router/src/lib.rs @@ -564,9 +564,7 @@ mod pallet { taker_amount: BalanceOf, ) -> Result { match strategy { - Strategy::ImmediateOrCancel => { - return Err(Error::::CancelStrategyApplied.into()); - } + Strategy::ImmediateOrCancel => Err(Error::::CancelStrategyApplied.into()), Strategy::LimitOrder => { match T::OrderBook::place_order( who.clone(), From a148ec7ca95e3de0024410178175771d4247b536 Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Thu, 4 Apr 2024 12:41:30 +0200 Subject: [PATCH 8/9] fix CI --- zrml/orderbook/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zrml/orderbook/src/tests.rs b/zrml/orderbook/src/tests.rs index aed88b2ef..d37b70c77 100644 --- a/zrml/orderbook/src/tests.rs +++ b/zrml/orderbook/src/tests.rs @@ -632,7 +632,7 @@ fn it_fills_order_fully_maker_base_asset() { assert_eq!(reserved_bob, 0); let external_fee_amount = calculate_fee::(maker_amount); - let external_fee = ExternalFee { account: BOB, amount: external_fee_amount }; + let external_fee = ExternalFee { account: ALICE, amount: external_fee_amount }; System::assert_last_event( Event::::OrderFilled { @@ -721,7 +721,7 @@ fn it_fills_order_partially_maker_base_asset() { } ); - let external_fee = ExternalFee { account: BOB, amount: external_fee_amount }; + let external_fee = ExternalFee { account: ALICE, amount: external_fee_amount }; System::assert_last_event( Event::::OrderFilled { From e89f24894103a8e1d86ce4887b0b701ded3be092 Mon Sep 17 00:00:00 2001 From: Chralt98 Date: Mon, 8 Apr 2024 09:09:22 +0200 Subject: [PATCH 9/9] remove swaps pallet dependency --- Cargo.lock | 1 - zrml/hybrid-router/Cargo.toml | 2 -- 2 files changed, 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ef242b4dc..ea3695b84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14622,7 +14622,6 @@ dependencies = [ "zrml-orderbook", "zrml-prediction-markets", "zrml-simple-disputes", - "zrml-swaps", ] [[package]] diff --git a/zrml/hybrid-router/Cargo.toml b/zrml/hybrid-router/Cargo.toml index a222f647a..18a8a3812 100644 --- a/zrml/hybrid-router/Cargo.toml +++ b/zrml/hybrid-router/Cargo.toml @@ -30,7 +30,6 @@ zrml-neo-swaps = { workspace = true, optional = true } zrml-orderbook = { workspace = true, optional = true } zrml-prediction-markets = { workspace = true, optional = true } zrml-simple-disputes = { workspace = true, optional = true } -zrml-swaps = { workspace = true, optional = true } [dev-dependencies] env_logger = { workspace = true } @@ -58,7 +57,6 @@ mock = [ "zrml-prediction-markets/default", "zrml-prediction-markets/mock", "zrml-simple-disputes/default", - "zrml-swaps/default", "zrml-authorized/default", "zrml-court/default", "zrml-global-disputes/default",