Add with_fees boolean parameter for Swaps getSpotPrices() (#878)

* Add with_fees boolean parameter for Swaps getSpotPrices() * Fix runtime build issue * Don't use with_fees flag for rikiddo in get_spot_price() * Add with_fees parameter to SwapsApiServer * Update changelog_for_devs.md * Fix format issues * Fix format issue
zeitgeistpm · Nov 24, 2022 · 7bddaf3 · 7bddaf3
1 parent eae4841
commit 7bddaf3
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 35 deletions.
diff --git a/docs/changelog_for_devs.md b/docs/changelog_for_devs.md
@@ -50,6 +50,9 @@
 - `edit_market` extrinsic added, which enables creator of the market to edit
   market. It has same parameters as `create_market` except market_creation, on
   success it returns `MarketEdited` event.
+- get_spot_price() RPC from Swaps support `with_fees` boolean parameter to
+  include/exclude swap_fees in spot price, Currently this flag only works for
+  CPMM.
 
 # v0.3.6
 

diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs
@@ -1418,8 +1418,9 @@ macro_rules! create_runtime_api {
                     pool_id: &PoolId,
                     asset_in: &Asset<MarketId>,
                     asset_out: &Asset<MarketId>,
+                    with_fees: bool,
                 ) -> SerdeWrapper<Balance> {
-                    SerdeWrapper(Swaps::get_spot_price(pool_id, asset_in, asset_out).ok().unwrap_or(0))
+                    SerdeWrapper(Swaps::get_spot_price(pool_id, asset_in, asset_out, with_fees).ok().unwrap_or(0))
                 }
 
                 fn pool_account_id(pool_id: &PoolId) -> AccountId {

diff --git a/zrml/swaps/rpc/src/lib.rs b/zrml/swaps/rpc/src/lib.rs
@@ -62,6 +62,7 @@ where
         asset_in: Asset<MarketId>,
         asset_out: Asset<MarketId>,
         at: Option<BlockHash>,
+        with_fees: bool,
     ) -> RpcResult<SerdeWrapper<Balance>>;
 
     #[method(name = "swaps_getSpotPrices")]
@@ -71,6 +72,7 @@ where
         asset_in: Asset<MarketId>,
         asset_out: Asset<MarketId>,
         blocks: Vec<BlockNumber>,
+        with_fees: bool,
     ) -> RpcResult<Vec<SerdeWrapper<Balance>>>;
 }
 
@@ -161,16 +163,18 @@ where
         asset_in: Asset<MarketId>,
         asset_out: Asset<MarketId>,
         at: Option<<Block as BlockT>::Hash>,
+        with_fees: bool,
     ) -> RpcResult<SerdeWrapper<Balance>> {
         let api = self.client.runtime_api();
         let at = BlockId::hash(at.unwrap_or_else(|| self.client.info().best_hash));
-        let res = api.get_spot_price(&at, &pool_id, &asset_in, &asset_out).map_err(|e| {
-            CallError::Custom(ErrorObject::owned(
-                Error::RuntimeError.into(),
-                "Unable to get spot price.",
-                Some(e.to_string()),
-            ))
-        })?;
+        let res =
+            api.get_spot_price(&at, &pool_id, &asset_in, &asset_out, with_fees).map_err(|e| {
+                CallError::Custom(ErrorObject::owned(
+                    Error::RuntimeError.into(),
+                    "Unable to get spot price.",
+                    Some(e.to_string()),
+                ))
+            })?;
         Ok(res)
     }
 
@@ -180,14 +184,16 @@ where
         asset_in: Asset<MarketId>,
         asset_out: Asset<MarketId>,
         blocks: Vec<NumberFor<Block>>,
+        with_fees: bool,
     ) -> RpcResult<Vec<SerdeWrapper<Balance>>> {
         let api = self.client.runtime_api();
         blocks
             .into_iter()
             .map(|block| {
                 let hash = BlockId::number(block);
-                let res =
-                    api.get_spot_price(&hash, &pool_id, &asset_in, &asset_out).map_err(|e| {
+                let res = api
+                    .get_spot_price(&hash, &pool_id, &asset_in, &asset_out, with_fees)
+                    .map_err(|e| {
                         CallError::Custom(ErrorObject::owned(
                             Error::RuntimeError.into(),
                             "Unable to get spot price.",

diff --git a/zrml/swaps/runtime-api/src/lib.rs b/zrml/swaps/runtime-api/src/lib.rs
@@ -31,6 +31,11 @@ sp_api::decl_runtime_apis! {
     {
         fn pool_shares_id(pool_id: PoolId) -> Asset<SerdeWrapper<MarketId>>;
         fn pool_account_id(pool_id: &PoolId) -> AccountId;
-        fn get_spot_price(pool_id: &PoolId, asset_in: &Asset<MarketId>, asset_out: &Asset<MarketId>) -> SerdeWrapper<Balance>;
+        fn get_spot_price(
+            pool_id: &PoolId,
+            asset_in: &Asset<MarketId>,
+            asset_out: &Asset<MarketId>,
+            with_fees: bool,
+        ) -> SerdeWrapper<Balance>;
     }
 }
diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs
@@ -1383,6 +1383,7 @@ mod pallet {
             pool_id: &PoolId,
             asset_in: &Asset<MarketIdOf<T>>,
             asset_out: &Asset<MarketIdOf<T>>,
+            with_fees: bool,
         ) -> Result<BalanceOf<T>, DispatchError> {
             let pool = Self::pool_by_id(*pool_id)?;
             ensure!(pool.assets.binary_search(asset_in).is_ok(), Error::<T>::AssetNotInPool);
@@ -1394,7 +1395,12 @@ mod pallet {
                 let balance_out = T::AssetManager::free_balance(*asset_out, &pool_account);
                 let in_weight = Self::pool_weight_rslt(&pool, asset_in)?;
                 let out_weight = Self::pool_weight_rslt(&pool, asset_out)?;
-                let swap_fee = pool.swap_fee.ok_or(Error::<T>::SwapFeeMissing)?;
+
+                let swap_fee = if with_fees {
+                    pool.swap_fee.ok_or(Error::<T>::SwapFeeMissing)?
+                } else {
+                    BalanceOf::<T>::zero()
+                };
 
                 return Ok(crate::math::calc_spot_price(
                     balance_in.saturated_into(),
@@ -1406,6 +1412,7 @@ mod pallet {
                 .saturated_into());
             }
 
+            // TODO(#880): For now rikiddo does not respect with_fees flag.
             // Price when using Rikiddo.
             ensure!(pool.pool_status == PoolStatus::Active, Error::<T>::PoolIsNotActive);
             let mut balances = Vec::new();

diff --git a/zrml/swaps/src/mock.rs b/zrml/swaps/src/mock.rs
@@ -236,8 +236,9 @@ sp_api::mock_impl_runtime_apis! {
             pool_id: &PoolId,
             asset_in: &Asset<MarketId>,
             asset_out: &Asset<MarketId>,
+            with_fees: bool,
         ) -> SerdeWrapper<Balance> {
-            SerdeWrapper(Swaps::get_spot_price(pool_id, asset_in, asset_out).ok().unwrap_or(0))
+            SerdeWrapper(Swaps::get_spot_price(pool_id, asset_in, asset_out, with_fees).ok().unwrap_or(0))
         }
 
         fn pool_account_id(pool_id: &PoolId) -> AccountIdTest {

diff --git a/zrml/swaps/src/tests.rs b/zrml/swaps/src/tests.rs
@@ -186,7 +186,7 @@ fn allows_the_full_user_lifecycle() {
         let asset_b_bal = Currencies::free_balance(ASSET_B, &ALICE);
 
         // swap_exact_amount_in
-        let spot_price = Swaps::get_spot_price(&0, &ASSET_A, &ASSET_B).unwrap();
+        let spot_price = Swaps::get_spot_price(&0, &ASSET_A, &ASSET_B, true).unwrap();
         assert_eq!(spot_price, _1);
 
         let pool_account = Swaps::pool_account_id(&0);
@@ -689,25 +689,26 @@ fn most_operations_fail_if_pool_is_clean() {
     });
 }
 
-#[test_case(_3, _3, _100, _100, 0, 10_000_000_000)]
-#[test_case(_3, _3, _100, _150, 0, 6_666_666_667)]
-#[test_case(_3, _4, _100, _100, 0, 13_333_333_333)]
-#[test_case(_3, _4, _100, _150, 0, 8_888_888_889)]
-#[test_case(_3, _6, _125, _150, 0, 16_666_666_667)]
-#[test_case(_3, _6, _125, _100, 0, 25_000_000_000)]
-#[test_case(_3, _3, _100, _100, _1_10, 11_111_111_111)]
-#[test_case(_3, _3, _100, _150, _1_10, 7_407_407_408)]
-#[test_case(_3, _4, _100, _100, _1_10, 14_814_814_814)]
-#[test_case(_3, _4, _100, _150, _1_10, 9_876_543_210)]
-#[test_case(_3, _6, _125, _150, _1_10, 18_518_518_519)]
-#[test_case(_3, _6, _125, _100, _1_10, 27_777_777_778)]
+#[test_case(_3, _3, _100, _100, 0, 10_000_000_000, 10_000_000_000)]
+#[test_case(_3, _3, _100, _150, 0, 6_666_666_667, 6_666_666_667)]
+#[test_case(_3, _4, _100, _100, 0, 13_333_333_333, 13_333_333_333)]
+#[test_case(_3, _4, _100, _150, 0, 8_888_888_889, 8_888_888_889)]
+#[test_case(_3, _6, _125, _150, 0, 16_666_666_667, 16_666_666_667)]
+#[test_case(_3, _6, _125, _100, 0, 25_000_000_000, 25_000_000_000)]
+#[test_case(_3, _3, _100, _100, _1_10, 11_111_111_111, 10_000_000_000)]
+#[test_case(_3, _3, _100, _150, _1_10, 7_407_407_408, 6_666_666_667)]
+#[test_case(_3, _4, _100, _100, _1_10, 14_814_814_814, 13_333_333_333)]
+#[test_case(_3, _4, _100, _150, _1_10, 9_876_543_210, 8_888_888_889)]
+#[test_case(_3, _6, _125, _150, _1_10, 18_518_518_519, 16_666_666_667)]
+#[test_case(_3, _6, _125, _100, _1_10, 27_777_777_778, 25_000_000_000)]
 fn get_spot_price_returns_correct_results_cpmm(
     weight_in: u128,
     weight_out: u128,
     balance_in: BalanceOf<Runtime>,
     balance_out: BalanceOf<Runtime>,
     swap_fee: BalanceOf<Runtime>,
-    expected_spot_price: BalanceOf<Runtime>,
+    expected_spot_price_with_fees: BalanceOf<Runtime>,
+    expected_spot_price_without_fees: BalanceOf<Runtime>,
 ) {
     ExtBuilder::default().build().execute_with(|| {
         // We always swap ASSET_A for ASSET_B, but we vary the weights, balances and swap fees.
@@ -734,8 +735,13 @@ fn get_spot_price_returns_correct_results_cpmm(
 
         let abs_tol = 100;
         assert_approx!(
-            Swaps::get_spot_price(&pool_id, &ASSET_A, &ASSET_B).unwrap(),
-            expected_spot_price,
+            Swaps::get_spot_price(&pool_id, &ASSET_A, &ASSET_B, true).unwrap(),
+            expected_spot_price_with_fees,
+            abs_tol,
+        );
+        assert_approx!(
+            Swaps::get_spot_price(&pool_id, &ASSET_A, &ASSET_B, false).unwrap(),
+            expected_spot_price_without_fees,
             abs_tol,
         );
     });
@@ -747,23 +753,24 @@ fn get_spot_price_returns_correct_results_rikiddo() {
         create_initial_pool(ScoringRule::RikiddoSigmoidFeeMarketEma, None, false);
         let pool_id = 0;
         assert_noop!(
-            Swaps::get_spot_price(&pool_id, &ASSETS[0], &ASSETS[0]),
+            Swaps::get_spot_price(&pool_id, &ASSETS[0], &ASSETS[0], true),
             crate::Error::<Runtime>::PoolIsNotActive
         );
         subsidize_and_start_rikiddo_pool(pool_id, &ALICE, 0);
 
         // Asset out, base currency in. Should receive about 1/3 -> price about 3
         let price_base_in =
-            Swaps::get_spot_price(&pool_id, &ASSETS[0], ASSETS.last().unwrap()).unwrap();
+            Swaps::get_spot_price(&pool_id, &ASSETS[0], ASSETS.last().unwrap(), true).unwrap();
         // Between 0.3 and 0.4
         assert!(price_base_in > 28 * BASE / 10 && price_base_in < 31 * BASE / 10);
         // Base currency in, asset out. Price about 3.
         let price_base_out =
-            Swaps::get_spot_price(&pool_id, ASSETS.last().unwrap(), &ASSETS[0]).unwrap();
+            Swaps::get_spot_price(&pool_id, ASSETS.last().unwrap(), &ASSETS[0], true).unwrap();
         // Between 2.9 and 3.1
         assert!(price_base_out > 3 * BASE / 10 && price_base_out < 4 * BASE / 10);
         // Asset in, asset out. Price about 1.
-        let price_asset_in_out = Swaps::get_spot_price(&pool_id, &ASSETS[0], &ASSETS[1]).unwrap();
+        let price_asset_in_out =
+            Swaps::get_spot_price(&pool_id, &ASSETS[0], &ASSETS[1], true).unwrap();
         // Between 0.9 and 1.1
         assert!(price_asset_in_out > 9 * BASE / 10 && price_asset_in_out < 11 * BASE / 10);
     });

diff --git a/zrml/swaps/src/utils.rs b/zrml/swaps/src/utils.rs
@@ -177,7 +177,8 @@ where
         ensure!(p.pool.bound(&p.asset_out), Error::<T>::AssetNotBound);
     }
 
-    let spot_price_before = Pallet::<T>::get_spot_price(&p.pool_id, &p.asset_in, &p.asset_out)?;
+    let spot_price_before =
+        Pallet::<T>::get_spot_price(&p.pool_id, &p.asset_in, &p.asset_out, true)?;
     if let Some(max_price) = p.max_price {
         ensure!(spot_price_before <= max_price, Error::<T>::BadLimitPrice);
     }
@@ -212,7 +213,8 @@ where
         }
     }
 
-    let spot_price_after = Pallet::<T>::get_spot_price(&p.pool_id, &p.asset_in, &p.asset_out)?;
+    let spot_price_after =
+        Pallet::<T>::get_spot_price(&p.pool_id, &p.asset_in, &p.asset_out, true)?;
 
     // Allow little tolerance
     match p.pool.scoring_rule {