From fd5f0ace3d452ed30e490088b94a850f4d9c7e18 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Wed, 24 Jan 2024 11:50:53 +0100 Subject: [PATCH 1/4] Replace hard-coded `u128` fo weights with balance types --- primitives/src/traits/swaps.rs | 3 +-- zrml/swaps/src/benchmarks.rs | 6 +++--- zrml/swaps/src/lib.rs | 35 ++++++++++++++++++---------------- zrml/swaps/src/types/pool.rs | 6 ++---- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/primitives/src/traits/swaps.rs b/primitives/src/traits/swaps.rs index e5c182701..74412ba0d 100644 --- a/primitives/src/traits/swaps.rs +++ b/primitives/src/traits/swaps.rs @@ -23,7 +23,6 @@ use frame_support::dispatch::{DispatchError, Weight}; pub trait Swaps { type Asset; type Balance; - // TODO(#1216): Add weight type which implements `Into` and `From` /// Creates an initial active pool. /// @@ -44,7 +43,7 @@ pub trait Swaps { assets: Vec, swap_fee: Self::Balance, amount: Self::Balance, - weights: Vec, + weights: Vec, ) -> Result; /// Close the specified pool. diff --git a/zrml/swaps/src/benchmarks.rs b/zrml/swaps/src/benchmarks.rs index d4d414be0..dd1dabcb9 100644 --- a/zrml/swaps/src/benchmarks.rs +++ b/zrml/swaps/src/benchmarks.rs @@ -72,7 +72,7 @@ fn bench_create_pool( caller: T::AccountId, asset_count: usize, opt_asset_amount: Option>, - opt_weights: Option>, + opt_weights: Option>>, open: bool, ) -> (u128, Vec>, BalanceOf) where @@ -169,7 +169,7 @@ benchmarks! { let balance: BalanceOf = LIQUIDITY.saturated_into(); let asset_amount_in: BalanceOf = balance.bmul(T::MaxInRatio::get()).unwrap(); let weight_in = T::MinWeight::get(); - let weight_out = 2 * weight_in; + let weight_out = weight_in * 2u8.into(); let mut weights = vec![weight_in; asset_count as usize]; weights[asset_count as usize - 1] = weight_out; let caller: T::AccountId = whitelisted_caller(); @@ -205,7 +205,7 @@ benchmarks! { let balance: BalanceOf = LIQUIDITY.saturated_into(); let asset_amount_out: BalanceOf = balance.bmul(T::MaxOutRatio::get()).unwrap(); let weight_out = T::MinWeight::get(); - let weight_in = 4 * weight_out; + let weight_in = weight_out * 4u8.into(); let mut weights = vec![weight_out; asset_count as usize]; weights[0] = weight_in; let caller: T::AccountId = whitelisted_caller(); diff --git a/zrml/swaps/src/lib.rs b/zrml/swaps/src/lib.rs index 4774d2ce6..50feb354e 100644 --- a/zrml/swaps/src/lib.rs +++ b/zrml/swaps/src/lib.rs @@ -211,7 +211,7 @@ mod pallet { ensure!(pool_amount <= mul, Error::::MaxInRatio); let asset_amount: BalanceOf = crate::math::calc_single_out_given_pool_in( asset_balance.saturated_into(), - Self::pool_weight_rslt(&pool, &asset)?, + Self::pool_weight_rslt(&pool, &asset)?.saturated_into(), total_supply.saturated_into(), pool.total_weight.saturated_into(), pool_amount.saturated_into(), @@ -372,7 +372,7 @@ mod pallet { ensure!(pool_amount <= mul, Error::::MaxOutRatio); let asset_amount: BalanceOf = crate::math::calc_single_in_given_pool_out( asset_balance.saturated_into(), - Self::pool_weight_rslt(&pool, &asset)?, + Self::pool_weight_rslt(&pool, &asset)?.saturated_into(), total_supply.saturated_into(), pool.total_weight.saturated_into(), pool_amount.saturated_into(), @@ -537,17 +537,17 @@ mod pallet { type MaxSwapFee: Get>; #[pallet::constant] - type MaxTotalWeight: Get; + type MaxTotalWeight: Get>; #[pallet::constant] - type MaxWeight: Get; + type MaxWeight: Get>; #[pallet::constant] /// The minimum amount of assets in a pool. type MinAssets: Get; #[pallet::constant] - type MinWeight: Get; + type MinWeight: Get>; /// The module identifier. #[pallet::constant] @@ -784,9 +784,9 @@ mod pallet { Ok(crate::math::calc_spot_price( balance_in.saturated_into(), - in_weight, + in_weight.saturated_into(), balance_out.saturated_into(), - out_weight, + out_weight.saturated_into(), swap_fee, )? .saturated_into()) @@ -938,7 +938,10 @@ mod pallet { }) } - fn pool_weight_rslt(pool: &PoolOf, asset: &AssetOf) -> Result> { + fn pool_weight_rslt( + pool: &PoolOf, + asset: &AssetOf, + ) -> Result, Error> { pool.weights.get(asset).cloned().ok_or(Error::::AssetNotBound) } @@ -980,7 +983,7 @@ mod pallet { assets: Vec>, swap_fee: BalanceOf, amount: BalanceOf, - weights: Vec, + weights: Vec>, ) -> Result { ensure!(assets.len() <= usize::from(T::MaxAssets::get()), Error::::TooManyAssets); ensure!(assets.len() >= usize::from(T::MinAssets::get()), Error::::TooFewAssets); @@ -988,7 +991,7 @@ mod pallet { let pool_shares_id = Self::pool_shares_id(next_pool_id); let pool_account = Self::pool_account_id(&next_pool_id); let mut map = BTreeMap::new(); - let mut total_weight = 0; + let mut total_weight: BalanceOf = Zero::zero(); let mut sorted_assets = assets.clone(); sorted_assets.sort(); let has_duplicates = sorted_assets @@ -1132,7 +1135,7 @@ mod pallet { pool_amount: |asset_balance: BalanceOf, total_supply: BalanceOf| { let pool_amount: BalanceOf = crate::math::calc_pool_in_given_single_out( asset_balance.saturated_into(), - Self::pool_weight_rslt(pool_ref, &asset)?, + Self::pool_weight_rslt(pool_ref, &asset)?.saturated_into(), total_supply.saturated_into(), pool_ref.total_weight.saturated_into(), asset_amount.saturated_into(), @@ -1190,7 +1193,7 @@ mod pallet { ensure!(asset_amount <= mul, Error::::MaxInRatio); let pool_amount: BalanceOf = crate::math::calc_pool_out_given_single_in( asset_balance.saturated_into(), - Self::pool_weight_rslt(pool_ref, &asset_in)?, + Self::pool_weight_rslt(pool_ref, &asset_in)?.saturated_into(), total_supply.saturated_into(), pool_ref.total_weight.saturated_into(), asset_amount.saturated_into(), @@ -1258,9 +1261,9 @@ mod pallet { ); let asset_amount_out: BalanceOf = crate::math::calc_out_given_in( balance_in.saturated_into(), - Self::pool_weight_rslt(&pool, &asset_in)?, + Self::pool_weight_rslt(&pool, &asset_in)?.saturated_into(), balance_out.saturated_into(), - Self::pool_weight_rslt(&pool, &asset_out)?, + Self::pool_weight_rslt(&pool, &asset_out)?.saturated_into(), asset_amount_in.saturated_into(), pool.swap_fee.saturated_into(), )? @@ -1329,9 +1332,9 @@ mod pallet { let balance_in = T::AssetManager::free_balance(asset_in, &pool_account_id); let asset_amount_in: BalanceOf = crate::math::calc_in_given_out( balance_in.saturated_into(), - Self::pool_weight_rslt(&pool, &asset_in)?, + Self::pool_weight_rslt(&pool, &asset_in)?.saturated_into(), balance_out.saturated_into(), - Self::pool_weight_rslt(&pool, &asset_out)?, + Self::pool_weight_rslt(&pool, &asset_out)?.saturated_into(), asset_amount_out.saturated_into(), pool.swap_fee.saturated_into(), )? diff --git a/zrml/swaps/src/types/pool.rs b/zrml/swaps/src/types/pool.rs index 79fe8c234..917fe4a47 100644 --- a/zrml/swaps/src/types/pool.rs +++ b/zrml/swaps/src/types/pool.rs @@ -30,15 +30,13 @@ impl Get for MaxAssets { } } -// TODO(#1213): Replace `u128` with `PoolWeight` type which implements `Into` and -// `From`! Or just replace it with `Balance`. #[derive(TypeInfo, Clone, Encode, Eq, Decode, MaxEncodedLen, PartialEq, RuntimeDebug)] pub struct Pool { pub assets: BoundedVec, pub status: PoolStatus, pub swap_fee: Balance, - pub total_weight: u128, - pub weights: BoundedBTreeMap, + pub total_weight: Balance, + pub weights: BoundedBTreeMap, } impl Pool From 753125eca9b45f024ebe31e6102ee2c91c6f8a0d Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Wed, 24 Jan 2024 12:09:07 +0100 Subject: [PATCH 2/4] Replace modulo operator with `checked_rem_res` --- primitives/src/math/checked_ops_res.rs | 19 ++++++++++++++++++- zrml/swaps/src/fixed.rs | 6 ++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/primitives/src/math/checked_ops_res.rs b/primitives/src/math/checked_ops_res.rs index 02e9ec3de..69981385f 100644 --- a/primitives/src/math/checked_ops_res.rs +++ b/primitives/src/math/checked_ops_res.rs @@ -18,7 +18,7 @@ use frame_support::dispatch::DispatchError; use num_traits::{checked_pow, One}; use sp_arithmetic::{ - traits::{CheckedAdd, CheckedDiv, CheckedMul, CheckedSub}, + traits::{CheckedAdd, CheckedDiv, CheckedMul, CheckedRem, CheckedSub}, ArithmeticError, }; @@ -57,6 +57,13 @@ where fn checked_pow_res(&self, exp: usize) -> Result; } +pub trait CheckedRemRes +where + Self: Sized, +{ + fn checked_rem_res(&self, other: &Self) -> Result; +} + impl CheckedAddRes for T where T: CheckedAdd, @@ -106,3 +113,13 @@ where checked_pow(*self, exp).ok_or(DispatchError::Arithmetic(ArithmeticError::Overflow)) } } + +impl CheckedRemRes for T +where + T: CheckedRem, +{ + #[inline] + fn checked_rem_res(&self, other: &Self) -> Result { + self.checked_rem(other).ok_or(DispatchError::Arithmetic(ArithmeticError::DivisionByZero)) + } +} diff --git a/zrml/swaps/src/fixed.rs b/zrml/swaps/src/fixed.rs index ee74c799b..cd4a77ee8 100644 --- a/zrml/swaps/src/fixed.rs +++ b/zrml/swaps/src/fixed.rs @@ -26,7 +26,9 @@ use frame_support::dispatch::DispatchError; use zeitgeist_primitives::{ constants::BASE, math::{ - checked_ops_res::{CheckedAddRes, CheckedDivRes, CheckedMulRes, CheckedSubRes}, + checked_ops_res::{ + CheckedAddRes, CheckedDivRes, CheckedMulRes, CheckedRemRes, CheckedSubRes, + }, fixed::{FixedDiv, FixedMul}, }, }; @@ -53,7 +55,7 @@ pub fn bsub_sign(a: u128, b: u128) -> Result<(u128, bool), DispatchError> { } pub fn bpowi(a: u128, n: u128) -> Result { - let mut z = if n % 2 != 0 { a } else { BASE }; + let mut z = if n.checked_rem_res(&2)? != 0 { a } else { BASE }; let mut b = a; let mut m = n.checked_div_res(&2)?; From 858e4b5f95b80fa428254047eb24f9e0003a18e1 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Wed, 7 Feb 2024 11:34:09 +0100 Subject: [PATCH 3/4] Update copyright notices --- primitives/src/math/checked_ops_res.rs | 2 +- zrml/swaps/src/fixed.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/src/math/checked_ops_res.rs b/primitives/src/math/checked_ops_res.rs index 69981385f..1265a6b2b 100644 --- a/primitives/src/math/checked_ops_res.rs +++ b/primitives/src/math/checked_ops_res.rs @@ -1,4 +1,4 @@ -// Copyright 2023 Forecasting Technologies LTD. +// Copyright 2023-2024 Forecasting Technologies LTD. // // This file is part of Zeitgeist. // diff --git a/zrml/swaps/src/fixed.rs b/zrml/swaps/src/fixed.rs index cd4a77ee8..4b76e8c10 100644 --- a/zrml/swaps/src/fixed.rs +++ b/zrml/swaps/src/fixed.rs @@ -1,4 +1,4 @@ -// Copyright 2023 Forecasting Technologies LTD. +// Copyright 2023-2024 Forecasting Technologies LTD. // Copyright 2021-2022 Zeitgeist PM LLC. // // This file is part of Zeitgeist. From 305c9010b9475e34c80ff9d7ceefc8b62e049b96 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Fri, 9 Feb 2024 16:55:07 +0100 Subject: [PATCH 4/4] Revert changes to modulo operations --- primitives/src/math/checked_ops_res.rs | 21 ++------------------- zrml/swaps/src/fixed.rs | 8 +++----- 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/primitives/src/math/checked_ops_res.rs b/primitives/src/math/checked_ops_res.rs index 1265a6b2b..02e9ec3de 100644 --- a/primitives/src/math/checked_ops_res.rs +++ b/primitives/src/math/checked_ops_res.rs @@ -1,4 +1,4 @@ -// Copyright 2023-2024 Forecasting Technologies LTD. +// Copyright 2023 Forecasting Technologies LTD. // // This file is part of Zeitgeist. // @@ -18,7 +18,7 @@ use frame_support::dispatch::DispatchError; use num_traits::{checked_pow, One}; use sp_arithmetic::{ - traits::{CheckedAdd, CheckedDiv, CheckedMul, CheckedRem, CheckedSub}, + traits::{CheckedAdd, CheckedDiv, CheckedMul, CheckedSub}, ArithmeticError, }; @@ -57,13 +57,6 @@ where fn checked_pow_res(&self, exp: usize) -> Result; } -pub trait CheckedRemRes -where - Self: Sized, -{ - fn checked_rem_res(&self, other: &Self) -> Result; -} - impl CheckedAddRes for T where T: CheckedAdd, @@ -113,13 +106,3 @@ where checked_pow(*self, exp).ok_or(DispatchError::Arithmetic(ArithmeticError::Overflow)) } } - -impl CheckedRemRes for T -where - T: CheckedRem, -{ - #[inline] - fn checked_rem_res(&self, other: &Self) -> Result { - self.checked_rem(other).ok_or(DispatchError::Arithmetic(ArithmeticError::DivisionByZero)) - } -} diff --git a/zrml/swaps/src/fixed.rs b/zrml/swaps/src/fixed.rs index 4b76e8c10..ee74c799b 100644 --- a/zrml/swaps/src/fixed.rs +++ b/zrml/swaps/src/fixed.rs @@ -1,4 +1,4 @@ -// Copyright 2023-2024 Forecasting Technologies LTD. +// Copyright 2023 Forecasting Technologies LTD. // Copyright 2021-2022 Zeitgeist PM LLC. // // This file is part of Zeitgeist. @@ -26,9 +26,7 @@ use frame_support::dispatch::DispatchError; use zeitgeist_primitives::{ constants::BASE, math::{ - checked_ops_res::{ - CheckedAddRes, CheckedDivRes, CheckedMulRes, CheckedRemRes, CheckedSubRes, - }, + checked_ops_res::{CheckedAddRes, CheckedDivRes, CheckedMulRes, CheckedSubRes}, fixed::{FixedDiv, FixedMul}, }, }; @@ -55,7 +53,7 @@ pub fn bsub_sign(a: u128, b: u128) -> Result<(u128, bool), DispatchError> { } pub fn bpowi(a: u128, n: u128) -> Result { - let mut z = if n.checked_rem_res(&2)? != 0 { a } else { BASE }; + let mut z = if n % 2 != 0 { a } else { BASE }; let mut b = a; let mut m = n.checked_div_res(&2)?;