From 691954ee40d2ba03466c64201900ef26f909d11e Mon Sep 17 00:00:00 2001 From: Marian Vanderka Date: Mon, 9 Dec 2024 12:37:35 +0100 Subject: [PATCH] fixe while loop causing crash & add u512 to avoid overflows for maxxed inputs --- pallets/market/src/benchmarking.rs | 20 +++---- pallets/market/src/lib.rs | 3 +- pallets/stable-swap/src/lib.rs | 42 +++++++++------ rollup/runtime/integration-test/src/market.rs | 52 ++++++++++++++++++- rollup/runtime/src/lib.rs | 2 +- 5 files changed, 89 insertions(+), 30 deletions(-) diff --git a/pallets/market/src/benchmarking.rs b/pallets/market/src/benchmarking.rs index c9061e6fb..32758d775 100644 --- a/pallets/market/src/benchmarking.rs +++ b/pallets/market/src/benchmarking.rs @@ -22,7 +22,7 @@ where T::Balance: From, { let native = T::NativeCurrencyId::get(); - let id = T::Currency::create(who, (1_000 * UNIT).into()).expect("is ok"); + let id = T::Currency::create(who, (1_000_000_000 * UNIT).into()).expect("is ok"); // create some default entries for stable swap to work let _ = T::AssetRegistry::create_pool_asset(id, native, native); id @@ -42,9 +42,9 @@ where SystemOrigin::Signed(who.clone()).into(), kind, asset1, - UNIT.into(), + (1_000_000 * UNIT).into(), asset2, - UNIT.into() + (UNIT / 1_000_000).into() )); (asset1, asset2, lp_token) @@ -88,9 +88,9 @@ mod benchmarks { SystemOrigin::Signed(caller.clone()), kind, asset1, - UNIT.into(), + (10 * UNIT).into(), asset2, - UNIT.into(), + 10.into(), ); let lp_supply = T::Currency::total_issuance(lp_token); @@ -98,7 +98,7 @@ mod benchmarks { Event::LiquidityMinted { who: caller, pool_id: lp_token, - amounts_provided: (UNIT.into(), UNIT.into()), + amounts_provided: ((10 * UNIT).into(), 10.into()), lp_token, lp_token_minted: lp_supply, total_supply: lp_supply, @@ -169,7 +169,7 @@ mod benchmarks { // xyk returns None in this case, does a swap internally let lp_minted = Market::::calculate_expected_lp_minted(pool_id, amounts) - .unwrap_or(4192957199889574550.into()); + .unwrap_or(2496237962221088530.into()); #[extrinsic_call] mint_liquidity_fixed_amounts( @@ -502,9 +502,9 @@ mod benchmarks { SystemOrigin::Signed(caller.clone()), kind, asset1, - UNIT.into(), + (10 * UNIT).into(), asset2, - UNIT.into(), + 10.into(), ); let lp_supply = T::Currency::total_issuance(lp_token); @@ -512,7 +512,7 @@ mod benchmarks { Event::LiquidityMinted { who: caller, pool_id: lp_token, - amounts_provided: (UNIT.into(), UNIT.into()), + amounts_provided: ((10 * UNIT).into(), 10.into()), lp_token, lp_token_minted: lp_supply, total_supply: lp_supply, diff --git a/pallets/market/src/lib.rs b/pallets/market/src/lib.rs index e4de2789e..cff078000 100644 --- a/pallets/market/src/lib.rs +++ b/pallets/market/src/lib.rs @@ -144,8 +144,7 @@ pub mod pallet { >; /// Apis for LP asset creation in asset registry - type AssetRegistry: AssetRegistryProviderTrait - + AssetRegistryInspect; + type AssetRegistry: AssetRegistryProviderTrait; /// List of tokens ids that are not allowed to be used at all type DisabledTokens: Contains; diff --git a/pallets/stable-swap/src/lib.rs b/pallets/stable-swap/src/lib.rs index f8785837b..56e338572 100644 --- a/pallets/stable-swap/src/lib.rs +++ b/pallets/stable-swap/src/lib.rs @@ -1824,23 +1824,35 @@ impl Mutate for Pallet { let assets = vec![asset_1, asset_2]; let ten = T::Balance::from(10_u32); - let exp = |amount: Self::Balance| { - let mut i = 0_usize; - let mut pow_10 = T::Balance::one() * ten; - while amount % pow_10 == Zero::zero() { - i += 1; - pow_10 *= ten; - } - i - }; + ensure!(!amount_1.is_zero(), Error::::InitialLiquidityZeroAmount); + ensure!(!amount_2.is_zero(), Error::::InitialLiquidityZeroAmount); - let exp1 = exp(amount_1); - let exp2 = exp(amount_2); - let min = exp1.min(exp2); - let exp = checked_pow(ten, min).ok_or(Error::::MathOverflow)?; + let (rate_1_mul, rate_2_mul) = if amount_1 == amount_2 { + (One::one(), One::one()) + } else { + let exp = |amount: Self::Balance| { + let mut i = 0_usize; + let mut pow_10 = T::Balance::one() * ten; + loop { + if amount % pow_10 != Zero::zero() { + break; + } + i += 1; + if amount / pow_10 < ten { + break; + } + pow_10 *= ten; + } + i + }; + + let exp1 = exp(amount_1); + let exp2 = exp(amount_2); + let min = exp1.min(exp2); + let exp = checked_pow(ten, min).ok_or(Error::::MathOverflow)?; - let rate_1_mul = amount_1 / exp; - let rate_2_mul = amount_2 / exp; + (amount_1 / exp, amount_2 / exp) + }; // the max rate cannot be more than 1e18 let precision = diff --git a/rollup/runtime/integration-test/src/market.rs b/rollup/runtime/integration-test/src/market.rs index 062dc4dba..cae0b294e 100644 --- a/rollup/runtime/integration-test/src/market.rs +++ b/rollup/runtime/integration-test/src/market.rs @@ -15,8 +15,8 @@ fn test_env() -> TestExternalities { balances: vec![ (AccountId::from(ALICE), NATIVE_ASSET_ID, 100 * UNIT), (AccountId::from(ALICE), ASSET_ID_1, 100 * UNIT), - (AccountId::from(ALICE), ASSET_ID_2, 100 * UNIT), - (AccountId::from(ALICE), ASSET_ID_3, 100 * UNIT), + (AccountId::from(ALICE), ASSET_ID_2, Balance::MAX), + (AccountId::from(ALICE), ASSET_ID_3, Balance::MAX), ], ..ExtBuilder::default() } @@ -112,6 +112,54 @@ fn create_pool_works() { }) } +#[test] +fn create_pool_works_ss_min_max() { + test_env().execute_with(|| { + let min = 1; + let max_round_to_decimal: u128 = 300000000000000000000000000000000000000 / 10; + + assert_ok!(Market::create_pool( + origin(), + PoolKind::StableSwap, + ASSET_ID_2, + min, + ASSET_ID_3, + min, + )); + + let max_rate = UNIT; // 1e18 + let max_rate_fail = UNIT * 10; // 1e19 + assert_ok!(Market::create_pool( + origin(), + PoolKind::StableSwap, + ASSET_ID_2, + min, + ASSET_ID_3, + max_rate, + )); + assert_err!( + Market::create_pool( + origin(), + PoolKind::StableSwap, + ASSET_ID_2, + min, + ASSET_ID_3, + max_rate_fail, + ), + pallet_stable_swap::Error::::InitialPoolRateOutOfRange + ); + + assert_ok!(Market::create_pool( + origin(), + PoolKind::StableSwap, + ASSET_ID_2, + max_round_to_decimal, + ASSET_ID_3, + max_round_to_decimal, + )); + }); +} + #[test] fn add_liquidity_works() { test_env().execute_with(|| { diff --git a/rollup/runtime/src/lib.rs b/rollup/runtime/src/lib.rs index 1486193b8..2bd24b52d 100644 --- a/rollup/runtime/src/lib.rs +++ b/rollup/runtime/src/lib.rs @@ -339,7 +339,7 @@ impl pallet_stable_swap::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Currency = orml_tokens::MultiTokenCurrencyAdapter; type Balance = Balance; - type HigherPrecisionBalance = sp_core::U256; + type HigherPrecisionBalance = sp_core::U512; type CurrencyId = TokenId; type TreasuryPalletId = cfg::TreasuryPalletIdOf; type BnbTreasurySubAccDerive = cfg::pallet_xyk::BnbTreasurySubAccDerive;