From 34700d83b2c83e908ccde6c5da06c6d933e5c992 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 14 Feb 2024 19:58:50 +0100 Subject: [PATCH 01/29] fix bug with ed vs router when nonnative balances falls below ED after route trade --- integration-tests/src/router.rs | 87 ++++++++++++++++++++++++++++++- pallets/route-executor/src/lib.rs | 19 ++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index f358ae078..f3ab93c48 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -727,6 +727,91 @@ mod omnipool_router_tests { }); } + #[test] + fn sell_should_work_when_user_has_left_less_than_existential_in_nonnative() { + TestNet::reset(); + + Hydra::execute_with(|| { + //Arrange + let (pool_id, stable_asset_1, _) = init_stableswap().unwrap(); + + init_omnipool(); + + let init_balance = 3000 * UNITS + 1; + assert_ok!(Currencies::update_balance( + hydradx_runtime::RuntimeOrigin::root(), + ALICE.into(), + stable_asset_1, + init_balance as i128, + )); + + let trades = vec![Trade { + pool: PoolType::Stableswap(pool_id), + asset_in: stable_asset_1, + asset_out: pool_id, + }]; + + assert_balance!(ALICE.into(), pool_id, 0); + + //Act + let amount_to_sell = 3000 * UNITS; + assert_ok!(Router::sell( + hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), + stable_asset_1, + pool_id, + amount_to_sell, + 0, + trades + )); + + //Assert + assert_eq!( + hydradx_runtime::Currencies::free_balance(stable_asset_1, &AccountId::from(ALICE)), + 0 + ); + }); + } + + #[test] + fn sell_should_work_when_user_has_left_less_than_existential_in_native() { + TestNet::reset(); + + Hydra::execute_with(|| { + //Arrange + init_omnipool(); + + assert_ok!(Currencies::update_balance( + hydradx_runtime::RuntimeOrigin::root(), + ALICE.into(), + HDX, + 1 as i128, + )); + + let trades = vec![Trade { + pool: PoolType::Omnipool, + asset_in: HDX, + asset_out: DAI, + }]; + + //Act and assert + let amount_to_sell = ALICE_INITIAL_NATIVE_BALANCE; + assert_ok!(Router::sell( + hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), + HDX, + DAI, + amount_to_sell, + 0, + trades + )); + + //Assert + assert_eq!( + hydradx_runtime::Currencies::free_balance(HDX, &AccountId::from(ALICE)), + 0 + ); + }); + } + #[test] fn sell_hub_asset_should_work_when_route_contains_single_trade() { TestNet::reset(); @@ -3373,7 +3458,7 @@ pub fn init_stableswap_with_liquidity( let mut asset_ids: Vec<::AssetId> = Vec::new(); for idx in 0u32..MAX_ASSETS_IN_POOL { let name: Vec = idx.to_ne_bytes().to_vec(); - let asset_id = AssetRegistry::create_asset(&name, 1u128)?; + let asset_id = AssetRegistry::create_asset(&name, 1000u128)?; AssetRegistry::set_metadata(hydradx_runtime::RuntimeOrigin::root(), asset_id, b"xDUM".to_vec(), 18u8)?; asset_ids.push(asset_id); diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index e9f026222..deb4c825c 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -37,7 +37,7 @@ pub use hydradx_traits::router::{ }; use orml_traits::arithmetic::{CheckedAdd, CheckedSub}; use sp_runtime::traits::{AccountIdConversion, CheckedDiv}; -use sp_runtime::{ArithmeticError, DispatchError, TransactionOutcome}; +use sp_runtime::{ArithmeticError, DispatchError, Saturating, TransactionOutcome}; use sp_std::{vec, vec::Vec}; #[cfg(test)] @@ -59,6 +59,7 @@ pub mod pallet { use frame_system::pallet_prelude::OriginFor; use hydradx_traits::router::ExecutorError; use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedDiv}; + use sp_runtime::Saturating; #[pallet::pallet] pub struct Pallet(_); @@ -80,7 +81,8 @@ pub mod pallet { + Default + CheckedSub + CheckedAdd - + CheckedDiv; + + CheckedDiv + + Saturating; /// Native Asset Id #[pallet::constant] @@ -204,6 +206,10 @@ pub mod pallet { for (trade_amount, trade) in trade_amounts.iter().zip(route) { let user_balance_of_asset_in_before_trade = T::Currency::reducible_balance(trade.asset_in, &who, Preservation::Preserve, Fortitude::Polite); + let user_balance_of_asset_in_before_trade_with_protecting = + T::Currency::reducible_balance(asset_in, &who, Preservation::Protect, Fortitude::Polite); + let ed = user_balance_of_asset_in_before_trade + .saturating_sub(user_balance_of_asset_in_before_trade_with_protecting); let execution_result = T::AMM::execute_sell( origin.clone(), @@ -221,6 +227,7 @@ pub mod pallet { trade.asset_in, user_balance_of_asset_in_before_trade, trade_amount.amount_in, + ed, )?; } @@ -309,6 +316,7 @@ pub mod pallet { asset_in, user_balance_of_asset_in_before_trade, last_trade_amount.amount_in, + u128::MIN.into(), //TODO: add test for buy then fix it, )?; Self::deposit_event(Event::RouteExecuted { @@ -445,11 +453,18 @@ impl Pallet { asset_in: T::AssetId, user_balance_of_asset_in_before_trade: T::Balance, spent_amount: T::Balance, + ed: T::Balance, ) -> Result<(), DispatchError> { + //TODO: we might not need this check anymore, verify it with test sell_should_work_when_user_has_left_less_than_existential_in_native and also other DCA test if spent_amount < user_balance_of_asset_in_before_trade { let user_balance_of_asset_in_after_trade = T::Currency::reducible_balance(asset_in, &who, Preservation::Preserve, Fortitude::Polite); + let expected_user_balance = user_balance_of_asset_in_before_trade.saturating_sub(spent_amount); + if expected_user_balance < ed { + return Ok(()); //The user had leftotver less than ED so wiped out, hence we can't check the balance precisely + } + ensure!( user_balance_of_asset_in_before_trade - spent_amount == user_balance_of_asset_in_after_trade, Error::::InvalidRouteExecution From ca326bdcea42751d401de316352dd7360c2b78a7 Mon Sep 17 00:00:00 2001 From: dmoka Date: Mon, 19 Feb 2024 12:09:48 +0100 Subject: [PATCH 02/29] fix router ed issue for sell for both native and nonnative --- pallets/route-executor/src/lib.rs | 33 ++++++++++++++++++------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index deb4c825c..887d44302 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -208,8 +208,13 @@ pub mod pallet { T::Currency::reducible_balance(trade.asset_in, &who, Preservation::Preserve, Fortitude::Polite); let user_balance_of_asset_in_before_trade_with_protecting = T::Currency::reducible_balance(asset_in, &who, Preservation::Protect, Fortitude::Polite); - let ed = user_balance_of_asset_in_before_trade - .saturating_sub(user_balance_of_asset_in_before_trade_with_protecting); + let ed = if trade.asset_in == T::NativeAssetId::get() { + user_balance_of_asset_in_before_trade_with_protecting + .saturating_sub(user_balance_of_asset_in_before_trade) + } else { + user_balance_of_asset_in_before_trade + .saturating_sub(user_balance_of_asset_in_before_trade_with_protecting) + }; let execution_result = T::AMM::execute_sell( origin.clone(), @@ -456,20 +461,20 @@ impl Pallet { ed: T::Balance, ) -> Result<(), DispatchError> { //TODO: we might not need this check anymore, verify it with test sell_should_work_when_user_has_left_less_than_existential_in_native and also other DCA test - if spent_amount < user_balance_of_asset_in_before_trade { - let user_balance_of_asset_in_after_trade = - T::Currency::reducible_balance(asset_in, &who, Preservation::Preserve, Fortitude::Polite); + //if spent_amount < user_balance_of_asset_in_before_trade { + let user_balance_of_asset_in_after_trade = + T::Currency::reducible_balance(asset_in, &who, Preservation::Preserve, Fortitude::Polite); - let expected_user_balance = user_balance_of_asset_in_before_trade.saturating_sub(spent_amount); - if expected_user_balance < ed { - return Ok(()); //The user had leftotver less than ED so wiped out, hence we can't check the balance precisely - } - - ensure!( - user_balance_of_asset_in_before_trade - spent_amount == user_balance_of_asset_in_after_trade, - Error::::InvalidRouteExecution - ); + let expected_user_balance = user_balance_of_asset_in_before_trade.saturating_sub(spent_amount); + if expected_user_balance < ed { + return Ok(()); //The user had leftover less than ED so wiped out, hence we can't check the balance precisely } + + ensure!( + user_balance_of_asset_in_before_trade - spent_amount == user_balance_of_asset_in_after_trade, + Error::::InvalidRouteExecution + ); + //} Ok(()) } From 21eef4d46fc565927a8b818c1007a17773c59b87 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 10:36:54 +0100 Subject: [PATCH 03/29] simplify the ED handling in router --- integration-tests/src/dca.rs | 9 +++++ integration-tests/src/router.rs | 5 ++- pallets/route-executor/src/lib.rs | 61 +++++++++++++++++++------------ 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/integration-tests/src/dca.rs b/integration-tests/src/dca.rs index 7956b8ec2..4529e20dd 100644 --- a/integration-tests/src/dca.rs +++ b/integration-tests/src/dca.rs @@ -2382,6 +2382,7 @@ fn crate_xyk_pool(asset_a: AssetId, amount_a: Balance, asset_b: AssetId, amount_ mod with_onchain_route { use super::*; use hydradx_traits::router::PoolType; + use sp_core::crypto::AccountId32; #[test] fn buy_should_work_with_omnipool_and_stable_with_onchain_routes() { @@ -2853,6 +2854,8 @@ mod with_onchain_route { TestNet::reset(); Hydra::execute_with(|| { + let ps = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); + //Arrange init_omnipol(); @@ -2865,6 +2868,8 @@ mod with_onchain_route { FixedU128::from_rational(50, 100), )); + let ps = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); + //Populate xyk assert_ok!(Currencies::update_balance( hydradx_runtime::RuntimeOrigin::root(), @@ -2903,6 +2908,8 @@ mod with_onchain_route { }, ]; + let ps2 = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); + let asset_pair = AssetPair::new(HDX, DOT); assert_ok!(Router::set_route( hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), @@ -2948,8 +2955,10 @@ mod with_onchain_route { TransactionOutcome::Rollback(Ok::(alice_received_dot)) }) .unwrap(); + let ps3 = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); create_schedule(ALICE, schedule); + let p4 = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); //Act set_relaychain_block_number(11); diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index 983357727..7557fecd0 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -726,14 +726,17 @@ mod omnipool_router_tests { .into()]); }); } - + use sp_runtime::AccountId32; #[test] fn sell_should_work_when_user_has_left_less_than_existential_in_nonnative() { TestNet::reset(); Hydra::execute_with(|| { //Arrange + let ps = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); + let (pool_id, stable_asset_1, _) = init_stableswap().unwrap(); + let ps2 = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); init_omnipool(); diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 618ff46cb..0acd6744d 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -28,6 +28,7 @@ use frame_support::{ traits::{fungibles::Inspect, Get}, transactional, }; +use sp_runtime::traits::Zero; use frame_system::pallet_prelude::OriginFor; use frame_system::{ensure_signed, Origin}; @@ -42,7 +43,6 @@ use sp_std::{vec, vec::Vec}; #[cfg(test)] mod tests; - pub mod weights; // Re-export pallet items so that they can be accessed from the crate namespace. @@ -58,7 +58,7 @@ pub mod pallet { use frame_support::traits::fungibles::Mutate; use frame_system::pallet_prelude::OriginFor; use hydradx_traits::router::ExecutorError; - use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedDiv}; + use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedDiv, Zero}; use sp_runtime::Saturating; #[pallet::pallet] @@ -82,7 +82,8 @@ pub mod pallet { + CheckedSub + CheckedAdd + CheckedDiv - + Saturating; + + Saturating + + Zero; /// Native Asset Id #[pallet::constant] @@ -207,16 +208,8 @@ pub mod pallet { for (trade_amount, trade) in trade_amounts.iter().zip(route) { let user_balance_of_asset_in_before_trade = - T::Currency::reducible_balance(trade.asset_in, &who, Preservation::Preserve, Fortitude::Polite); - let user_balance_of_asset_in_before_trade_with_protecting = - T::Currency::reducible_balance(asset_in, &who, Preservation::Protect, Fortitude::Polite); - let ed = if trade.asset_in == T::NativeAssetId::get() { - user_balance_of_asset_in_before_trade_with_protecting - .saturating_sub(user_balance_of_asset_in_before_trade) - } else { - user_balance_of_asset_in_before_trade - .saturating_sub(user_balance_of_asset_in_before_trade_with_protecting) - }; + T::Currency::reducible_balance(trade.asset_in, &who, Preservation::Expendable, Fortitude::Polite); + let existential_deposit = Self::get_existential_deposit(&who, asset_in); let execution_result = T::AMM::execute_sell( origin.clone(), @@ -234,7 +227,7 @@ pub mod pallet { trade.asset_in, user_balance_of_asset_in_before_trade, trade_amount.amount_in, - ed, + existential_deposit, )?; } @@ -287,7 +280,8 @@ pub mod pallet { Self::ensure_route_arguments(&asset_pair, &route)?; let user_balance_of_asset_in_before_trade = - T::Currency::reducible_balance(asset_in, &who, Preservation::Preserve, Fortitude::Polite); + T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); + let existential_deposit = Self::get_existential_deposit(&who, asset_in); let trade_amounts = Self::calculate_buy_trade_amounts(&route, amount_out)?; @@ -322,7 +316,7 @@ pub mod pallet { asset_in, user_balance_of_asset_in_before_trade, first_trade.amount_in, - u128::MIN.into(), //TODO: add test for buy then fix it, + existential_deposit, //TODO: add test for buy then fix it, )?; Self::deposit_event(Event::Executed { @@ -467,23 +461,23 @@ impl Pallet { asset_in: T::AssetId, user_balance_of_asset_in_before_trade: T::Balance, spent_amount: T::Balance, - ed: T::Balance, + existential_deposit: T::Balance, ) -> Result<(), DispatchError> { - //TODO: we might not need this check anymore, verify it with test sell_should_work_when_user_has_left_less_than_existential_in_native and also other DCA test - //if spent_amount < user_balance_of_asset_in_before_trade { let user_balance_of_asset_in_after_trade = - T::Currency::reducible_balance(asset_in, &who, Preservation::Preserve, Fortitude::Polite); + T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); - let expected_user_balance = user_balance_of_asset_in_before_trade.saturating_sub(spent_amount); - if expected_user_balance < ed { + let expected_user_balance_of_asset_in_after_trade = user_balance_of_asset_in_before_trade + .checked_sub(&spent_amount) + .ok_or(Error::::InvalidRouteExecution)?; + + if expected_user_balance_of_asset_in_after_trade < existential_deposit { return Ok(()); //The user had leftover less than ED so wiped out, hence we can't check the balance precisely } ensure!( - user_balance_of_asset_in_before_trade - spent_amount == user_balance_of_asset_in_after_trade, + expected_user_balance_of_asset_in_after_trade == user_balance_of_asset_in_after_trade, Error::::InvalidRouteExecution ); - //} Ok(()) } @@ -540,7 +534,11 @@ impl Pallet { with_transaction(|| { let origin: OriginFor = Origin::::Signed(Self::router_account()).into(); + let ps1 = frame_system::Pallet::::providers(&Self::router_account()); + let _ = T::Currency::mint_into(asset_in, &Self::router_account(), amount_in); + //TODO: remove - tge ref count increases by 1 as we do try_mutate_account from pallet balances + let ps2 = frame_system::Pallet::::providers(&Self::router_account()); let sell_result = Self::sell(origin, asset_in, asset_out, amount_in, u128::MIN.into(), route.clone()); @@ -619,6 +617,21 @@ impl Pallet { Ok(Pays::No.into()) } + + fn get_existential_deposit(who: &T::AccountId, asset: T::AssetId) -> T::Balance { + let user_balance_of_asset_in_before_trade2 = + T::Currency::reducible_balance(asset, &who, Preservation::Preserve, Fortitude::Polite); + let user_balance_of_asset_in_before_trade_with_protecting = + T::Currency::reducible_balance(asset, &who, Preservation::Protect, Fortitude::Polite); + + let ed = if asset == T::NativeAssetId::get() { + user_balance_of_asset_in_before_trade_with_protecting.saturating_sub(user_balance_of_asset_in_before_trade2) + } else { + user_balance_of_asset_in_before_trade2.saturating_sub(user_balance_of_asset_in_before_trade_with_protecting) + }; + + ed + } } impl RouterT, AmountInAndOut> From ccc9125f2ae84e624bfe01f6f9c24e5a58aa5e78 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 10:59:26 +0100 Subject: [PATCH 04/29] remove unnecessary variables --- integration-tests/src/dca.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/integration-tests/src/dca.rs b/integration-tests/src/dca.rs index 4529e20dd..e45cc4f90 100644 --- a/integration-tests/src/dca.rs +++ b/integration-tests/src/dca.rs @@ -2854,8 +2854,6 @@ mod with_onchain_route { TestNet::reset(); Hydra::execute_with(|| { - let ps = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); - //Arrange init_omnipol(); @@ -2868,8 +2866,6 @@ mod with_onchain_route { FixedU128::from_rational(50, 100), )); - let ps = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); - //Populate xyk assert_ok!(Currencies::update_balance( hydradx_runtime::RuntimeOrigin::root(), @@ -2908,8 +2904,6 @@ mod with_onchain_route { }, ]; - let ps2 = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); - let asset_pair = AssetPair::new(HDX, DOT); assert_ok!(Router::set_route( hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), @@ -2955,10 +2949,8 @@ mod with_onchain_route { TransactionOutcome::Rollback(Ok::(alice_received_dot)) }) .unwrap(); - let ps3 = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); create_schedule(ALICE, schedule); - let p4 = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); //Act set_relaychain_block_number(11); From 4af2218d66c6cfadee90bffe05a0412db67857c1 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 11:01:02 +0100 Subject: [PATCH 05/29] remove unnecessary variables --- integration-tests/src/router.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index 7557fecd0..15963dd1a 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -733,10 +733,7 @@ mod omnipool_router_tests { Hydra::execute_with(|| { //Arrange - let ps = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); - let (pool_id, stable_asset_1, _) = init_stableswap().unwrap(); - let ps2 = frame_system::Pallet::::providers(&AccountId32::from(ALICE)); init_omnipool(); From 421acf263c305816a44a1aa3fe17b345dd0c2818 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 11:02:48 +0100 Subject: [PATCH 06/29] remove unnecessary variables --- pallets/route-executor/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 0acd6744d..401e32514 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -534,11 +534,7 @@ impl Pallet { with_transaction(|| { let origin: OriginFor = Origin::::Signed(Self::router_account()).into(); - let ps1 = frame_system::Pallet::::providers(&Self::router_account()); - let _ = T::Currency::mint_into(asset_in, &Self::router_account(), amount_in); - //TODO: remove - tge ref count increases by 1 as we do try_mutate_account from pallet balances - let ps2 = frame_system::Pallet::::providers(&Self::router_account()); let sell_result = Self::sell(origin, asset_in, asset_out, amount_in, u128::MIN.into(), route.clone()); From ccee548e8cb00a54f7095249a05354091c46d1e2 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 11:07:44 +0100 Subject: [PATCH 07/29] simlify balance check --- pallets/route-executor/src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 401e32514..11e5fafb4 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -470,12 +470,10 @@ impl Pallet { .checked_sub(&spent_amount) .ok_or(Error::::InvalidRouteExecution)?; - if expected_user_balance_of_asset_in_after_trade < existential_deposit { - return Ok(()); //The user had leftover less than ED so wiped out, hence we can't check the balance precisely - } - + //If the user had leftover less than ED then it is wiped out, hence we can't check the balance precisely ensure!( - expected_user_balance_of_asset_in_after_trade == user_balance_of_asset_in_after_trade, + expected_user_balance_of_asset_in_after_trade < existential_deposit + || expected_user_balance_of_asset_in_after_trade == user_balance_of_asset_in_after_trade, Error::::InvalidRouteExecution ); Ok(()) From 23de687334ed3ea625850aa52725f5ec14e223aa Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 11:16:18 +0100 Subject: [PATCH 08/29] buy should work for the case when the user has less then ED afre the trade so precise balance check is not possible --- integration-tests/src/router.rs | 33 +++++++++++++++++++++++++++++++ pallets/route-executor/src/lib.rs | 2 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index 15963dd1a..a167a395b 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -972,6 +972,39 @@ mod omnipool_router_tests { }); } + #[test] + fn buy_should_work_when_after_trade_reamining_balance_is_less_than_existential_deposit() { + TestNet::reset(); + + Hydra::execute_with(|| { + //Arrange + init_omnipool(); + + let amount_to_buy = 26579363534770086553u128; + let amount_to_buy = 26559360000000000000u128; + + let limit = ALICE_INITIAL_NATIVE_BALANCE; + let trades = vec![Trade { + pool: PoolType::Omnipool, + asset_in: HDX, + asset_out: DAI, + }]; + + //Act + assert_ok!(Router::buy( + RuntimeOrigin::signed(BOB.into()), + HDX, + DAI, + amount_to_buy, + limit, + trades + )); + + //Assert + assert_balance!(BOB.into(), DAI, BOB_INITIAL_DAI_BALANCE + amount_to_buy); + }); + } + #[test] fn buy_hub_asset_should_not_work() { TestNet::reset(); diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 11e5fafb4..4bec0608a 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -316,7 +316,7 @@ pub mod pallet { asset_in, user_balance_of_asset_in_before_trade, first_trade.amount_in, - existential_deposit, //TODO: add test for buy then fix it, + existential_deposit, )?; Self::deposit_event(Event::Executed { From 5149c54c614578ecb9d0df050e013524e5a06131 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 14:30:27 +0100 Subject: [PATCH 09/29] bump versions --- Cargo.lock | 6 +++--- integration-tests/Cargo.toml | 2 +- pallets/route-executor/Cargo.toml | 2 +- runtime/hydradx/Cargo.toml | 2 +- runtime/hydradx/src/lib.rs | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b40684741..f3167bef4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4619,7 +4619,7 @@ dependencies = [ [[package]] name = "hydradx-runtime" -version = "208.0.0" +version = "209.0.0" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", @@ -8259,7 +8259,7 @@ dependencies = [ [[package]] name = "pallet-route-executor" -version = "2.0.0" +version = "2.0.1" dependencies = [ "frame-benchmarking", "frame-support", @@ -11302,7 +11302,7 @@ dependencies = [ [[package]] name = "runtime-integration-tests" -version = "1.17.4" +version = "1.17.5" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 268ef868a..a844516db 100644 --- a/integration-tests/Cargo.toml +++ b/integration-tests/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "runtime-integration-tests" -version = "1.17.4" +version = "1.17.5" description = "Integration tests" authors = ["GalacticCouncil"] edition = "2021" diff --git a/pallets/route-executor/Cargo.toml b/pallets/route-executor/Cargo.toml index 4ffa6cdd8..ca5b39e02 100644 --- a/pallets/route-executor/Cargo.toml +++ b/pallets/route-executor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-route-executor' -version = '2.0.0' +version = '2.0.1' description = 'A pallet to execute a route containing a sequence of trades' authors = ['GalacticCouncil'] edition = '2021' diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index 11db86bab..d3642e2a7 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "208.0.0" +version = "209.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index b4e80b0dc..0b574f675 100644 --- a/runtime/hydradx/src/lib.rs +++ b/runtime/hydradx/src/lib.rs @@ -107,7 +107,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("hydradx"), impl_name: create_runtime_str!("hydradx"), authoring_version: 1, - spec_version: 208, + spec_version: 209, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From c6985100d0e2399fd94c0b2fbd4a27b3495adcdd Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 15:15:18 +0100 Subject: [PATCH 10/29] make clippy happy --- integration-tests/src/dca.rs | 1 - integration-tests/src/router.rs | 4 +--- pallets/route-executor/src/lib.rs | 11 ++++------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/integration-tests/src/dca.rs b/integration-tests/src/dca.rs index e45cc4f90..7956b8ec2 100644 --- a/integration-tests/src/dca.rs +++ b/integration-tests/src/dca.rs @@ -2382,7 +2382,6 @@ fn crate_xyk_pool(asset_a: AssetId, amount_a: Balance, asset_b: AssetId, amount_ mod with_onchain_route { use super::*; use hydradx_traits::router::PoolType; - use sp_core::crypto::AccountId32; #[test] fn buy_should_work_with_omnipool_and_stable_with_onchain_routes() { diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index a167a395b..6543283c0 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -726,7 +726,6 @@ mod omnipool_router_tests { .into()]); }); } - use sp_runtime::AccountId32; #[test] fn sell_should_work_when_user_has_left_less_than_existential_in_nonnative() { TestNet::reset(); @@ -784,7 +783,7 @@ mod omnipool_router_tests { hydradx_runtime::RuntimeOrigin::root(), ALICE.into(), HDX, - 1 as i128, + 1_i128, )); let trades = vec![Trade { @@ -980,7 +979,6 @@ mod omnipool_router_tests { //Arrange init_omnipool(); - let amount_to_buy = 26579363534770086553u128; let amount_to_buy = 26559360000000000000u128; let limit = ALICE_INITIAL_NATIVE_BALANCE; diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 4bec0608a..536376533 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -28,7 +28,6 @@ use frame_support::{ traits::{fungibles::Inspect, Get}, transactional, }; -use sp_runtime::traits::Zero; use frame_system::pallet_prelude::OriginFor; use frame_system::{ensure_signed, Origin}; @@ -614,17 +613,15 @@ impl Pallet { fn get_existential_deposit(who: &T::AccountId, asset: T::AssetId) -> T::Balance { let user_balance_of_asset_in_before_trade2 = - T::Currency::reducible_balance(asset, &who, Preservation::Preserve, Fortitude::Polite); + T::Currency::reducible_balance(asset, who, Preservation::Preserve, Fortitude::Polite); let user_balance_of_asset_in_before_trade_with_protecting = - T::Currency::reducible_balance(asset, &who, Preservation::Protect, Fortitude::Polite); + T::Currency::reducible_balance(asset, who, Preservation::Protect, Fortitude::Polite); - let ed = if asset == T::NativeAssetId::get() { + if asset == T::NativeAssetId::get() { user_balance_of_asset_in_before_trade_with_protecting.saturating_sub(user_balance_of_asset_in_before_trade2) } else { user_balance_of_asset_in_before_trade2.saturating_sub(user_balance_of_asset_in_before_trade_with_protecting) - }; - - ed + } } } From b197e80292e17e84d0563795c1d9896441455229 Mon Sep 17 00:00:00 2001 From: dmoka Date: Tue, 20 Feb 2024 15:19:54 +0100 Subject: [PATCH 11/29] bump versions --- Cargo.lock | 4 ++-- integration-tests/Cargo.toml | 2 +- runtime/hydradx/Cargo.toml | 2 +- runtime/hydradx/src/lib.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f8533c355..eb85c8f1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4619,7 +4619,7 @@ dependencies = [ [[package]] name = "hydradx-runtime" -version = "209.0.0" +version = "210.0.0" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", @@ -11330,7 +11330,7 @@ dependencies = [ [[package]] name = "runtime-integration-tests" -version = "1.17.5" +version = "1.17.6" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 808800428..540b323bf 100644 --- a/integration-tests/Cargo.toml +++ b/integration-tests/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "runtime-integration-tests" -version = "1.17.5" +version = "1.17.6" description = "Integration tests" authors = ["GalacticCouncil"] edition = "2021" diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index c56f86ac4..199149c65 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "209.0.0" +version = "210.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index d1bd56142..1f2832c12 100644 --- a/runtime/hydradx/src/lib.rs +++ b/runtime/hydradx/src/lib.rs @@ -107,7 +107,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("hydradx"), impl_name: create_runtime_str!("hydradx"), authoring_version: 1, - spec_version: 209, + spec_version: 210, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From 3db469d4aafb69d2b9a58ff544497e811957c672 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 14:35:30 +0100 Subject: [PATCH 12/29] use asset registry to get ED --- pallets/asset-registry/src/lib.rs | 10 ++++++++ pallets/dca/src/tests/mock.rs | 9 +++++++ pallets/route-executor/src/lib.rs | 30 +++++++++++------------- pallets/route-executor/src/tests/mock.rs | 11 ++++++++- runtime/adapters/src/tests/mock.rs | 8 +++++++ runtime/hydradx/src/assets.rs | 1 + 6 files changed, 52 insertions(+), 17 deletions(-) diff --git a/pallets/asset-registry/src/lib.rs b/pallets/asset-registry/src/lib.rs index c32b11d64..902408223 100644 --- a/pallets/asset-registry/src/lib.rs +++ b/pallets/asset-registry/src/lib.rs @@ -613,6 +613,16 @@ impl GetByKey for Pallet { } } +/// Allows querying the ED for an asset by its id. +pub struct ExistentialDepositGetter(PhantomData); +/// Allows querying the ED for an asset by its id. +/// An unknown asset will return `None`. +impl GetByKey> for ExistentialDepositGetter { + fn get(k: &T::AssetId) -> Option { + Pallet::::assets(k).and_then(|details| Some(details.existential_deposit)) + } +} + /// Allows querying the XCM rate limit for an asset by its id. pub struct XcmRateLimitsInRegistry(PhantomData); /// Allows querying the XCM rate limit for an asset by its id. diff --git a/pallets/dca/src/tests/mock.rs b/pallets/dca/src/tests/mock.rs index 2caeb9b67..ce3752203 100644 --- a/pallets/dca/src/tests/mock.rs +++ b/pallets/dca/src/tests/mock.rs @@ -349,6 +349,14 @@ parameter_types! { type Pools = (OmniPool, Xyk); +pub struct MockedExistentialDepositGetter; + +impl GetByKey> for MockedExistentialDepositGetter { + fn get(_: &AssetId) -> Option { + Some(1000) + } +} + impl pallet_route_executor::Config for Test { type RuntimeEvent = RuntimeEvent; type AssetId = AssetId; @@ -356,6 +364,7 @@ impl pallet_route_executor::Config for Test { type NativeAssetId = NativeCurrencyId; type Currency = FungibleCurrencies; type AMM = Pools; + type ExistentialDepositGetter = MockedExistentialDepositGetter; type DefaultRoutePoolType = DefaultRoutePoolType; type WeightInfo = (); } diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 536376533..b43459443 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -57,6 +57,7 @@ pub mod pallet { use frame_support::traits::fungibles::Mutate; use frame_system::pallet_prelude::OriginFor; use hydradx_traits::router::ExecutorError; + use orml_traits::GetByKey; use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedDiv, Zero}; use sp_runtime::Saturating; @@ -92,6 +93,8 @@ pub mod pallet { type Currency: Inspect + Mutate; + type ExistentialDepositGetter: GetByKey>; + /// Handlers for AMM pools to calculate and execute trades type AMM: TradeExecution< ::RuntimeOrigin, @@ -142,6 +145,8 @@ pub mod pallet { InvalidRoute, ///The route update was not successful RouteUpdateIsNotSuccessful, + ///No existential deposit found for asset + NoExistentialDeposit, } /// Storing routes for asset pairs @@ -208,7 +213,13 @@ pub mod pallet { for (trade_amount, trade) in trade_amounts.iter().zip(route) { let user_balance_of_asset_in_before_trade = T::Currency::reducible_balance(trade.asset_in, &who, Preservation::Expendable, Fortitude::Polite); - let existential_deposit = Self::get_existential_deposit(&who, asset_in); + let existential_deposit = + T::ExistentialDepositGetter::get(&trade.asset_in).ok_or(Error::::NoExistentialDeposit)?; + + ensure!( + user_balance_of_asset_in_before_trade >= trade_amount.amount_in, + Error::::InsufficientBalance + ); let execution_result = T::AMM::execute_sell( origin.clone(), @@ -280,8 +291,8 @@ pub mod pallet { let user_balance_of_asset_in_before_trade = T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); - let existential_deposit = Self::get_existential_deposit(&who, asset_in); - + let existential_deposit = + T::ExistentialDepositGetter::get(&asset_in).ok_or(Error::::NoExistentialDeposit)?; let trade_amounts = Self::calculate_buy_trade_amounts(&route, amount_out)?; let first_trade = trade_amounts.last().ok_or(Error::::RouteCalculationFailed)?; @@ -610,19 +621,6 @@ impl Pallet { Ok(Pays::No.into()) } - - fn get_existential_deposit(who: &T::AccountId, asset: T::AssetId) -> T::Balance { - let user_balance_of_asset_in_before_trade2 = - T::Currency::reducible_balance(asset, who, Preservation::Preserve, Fortitude::Polite); - let user_balance_of_asset_in_before_trade_with_protecting = - T::Currency::reducible_balance(asset, who, Preservation::Protect, Fortitude::Polite); - - if asset == T::NativeAssetId::get() { - user_balance_of_asset_in_before_trade_with_protecting.saturating_sub(user_balance_of_asset_in_before_trade2) - } else { - user_balance_of_asset_in_before_trade2.saturating_sub(user_balance_of_asset_in_before_trade_with_protecting) - } - } } impl RouterT, AmountInAndOut> diff --git a/pallets/route-executor/src/tests/mock.rs b/pallets/route-executor/src/tests/mock.rs index 356399674..25293c5e3 100644 --- a/pallets/route-executor/src/tests/mock.rs +++ b/pallets/route-executor/src/tests/mock.rs @@ -24,6 +24,7 @@ use frame_support::{ use frame_system::{ensure_signed, pallet_prelude::OriginFor}; use hydradx_traits::router::{ExecutorError, PoolType, TradeExecution}; use orml_traits::parameter_type_with_key; +use orml_traits::GetByKey; use pallet_currencies::{fungibles::FungibleCurrencies, BasicCurrencyAdapter}; use pretty_assertions::assert_eq; use sp_core::H256; @@ -31,7 +32,6 @@ use sp_runtime::{ traits::{BlakeTwo256, IdentityLookup}, BuildStorage, DispatchError, }; - use std::cell::RefCell; use std::ops::Deref; @@ -146,11 +146,20 @@ impl Config for Test { type Balance = Balance; type NativeAssetId = NativeCurrencyId; type Currency = FungibleCurrencies; + type ExistentialDepositGetter = MockedExistentialDepositGetter; type AMM = Pools; type DefaultRoutePoolType = DefaultRoutePoolType; type WeightInfo = (); } +pub struct MockedExistentialDepositGetter; + +impl GetByKey> for MockedExistentialDepositGetter { + fn get(_: &AssetId) -> Option { + Some(1000) + } +} + pub type AccountId = u64; pub const ALICE: AccountId = 1; diff --git a/runtime/adapters/src/tests/mock.rs b/runtime/adapters/src/tests/mock.rs index 5eee69147..fdaa94edf 100644 --- a/runtime/adapters/src/tests/mock.rs +++ b/runtime/adapters/src/tests/mock.rs @@ -315,12 +315,20 @@ parameter_types! { type Pools = (Omnipool, XYK); +pub struct MockedExistentialDepositGetter; + +impl GetByKey> for MockedExistentialDepositGetter { + fn get(_: &AssetId) -> Option { + Some(1000) + } +} impl pallet_route_executor::Config for Test { type RuntimeEvent = RuntimeEvent; type AssetId = AssetId; type Balance = Balance; type NativeAssetId = NativeCurrencyId; type Currency = FungibleCurrencies; + type ExistentialDepositGetter = MockedExistentialDepositGetter; type AMM = Pools; type DefaultRoutePoolType = DefaultRoutePoolType; type WeightInfo = (); diff --git a/runtime/hydradx/src/assets.rs b/runtime/hydradx/src/assets.rs index 444f06d68..db21d87e3 100644 --- a/runtime/hydradx/src/assets.rs +++ b/runtime/hydradx/src/assets.rs @@ -778,6 +778,7 @@ impl pallet_route_executor::Config for Runtime { type AMM = (Omnipool, Stableswap, XYK, LBP); type DefaultRoutePoolType = DefaultRoutePoolType; type NativeAssetId = NativeAssetId; + type ExistentialDepositGetter = pallet_asset_registry::ExistentialDepositGetter; } parameter_types! { From 4cbbc7b44ed181a5515260ab9218e26a6c244054 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 14:37:14 +0100 Subject: [PATCH 13/29] remove mistakenlty plced balancecheck --- pallets/route-executor/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index b43459443..230fe78fd 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -216,11 +216,6 @@ pub mod pallet { let existential_deposit = T::ExistentialDepositGetter::get(&trade.asset_in).ok_or(Error::::NoExistentialDeposit)?; - ensure!( - user_balance_of_asset_in_before_trade >= trade_amount.amount_in, - Error::::InsufficientBalance - ); - let execution_result = T::AMM::execute_sell( origin.clone(), trade.pool, From a377f7320ef9aba82598e8b1065c929ca8d3566f Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 14:41:43 +0100 Subject: [PATCH 14/29] fix compilation error --- Cargo.lock | 2 +- pallets/asset-registry/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f26412c9..78678554f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11331,7 +11331,7 @@ dependencies = [ [[package]] name = "runtime-integration-tests" -version = "1.18.0" +version = "1.18.1" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", diff --git a/pallets/asset-registry/src/lib.rs b/pallets/asset-registry/src/lib.rs index 377affb97..dcb1bd16a 100644 --- a/pallets/asset-registry/src/lib.rs +++ b/pallets/asset-registry/src/lib.rs @@ -642,8 +642,8 @@ impl GetByKey for Pallet { pub struct ExistentialDepositGetter(PhantomData); /// Allows querying the ED for an asset by its id. /// An unknown asset will return `None`. -impl GetByKey> for ExistentialDepositGetter { - fn get(k: &T::AssetId) -> Option { +impl GetByKey> for ExistentialDepositGetter { + fn get(k: &T::AssetId) -> Option { Pallet::::assets(k).and_then(|details| Some(details.existential_deposit)) } } From 8c82f5a6097d7cec5ad8a3e469bc11ad8b526ada Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 14:45:53 +0100 Subject: [PATCH 15/29] bump versions --- Cargo.lock | 10 +++++----- pallets/asset-registry/Cargo.toml | 2 +- pallets/dca/Cargo.toml | 2 +- pallets/route-executor/Cargo.toml | 2 +- runtime/adapters/Cargo.toml | 2 +- runtime/hydradx/Cargo.toml | 2 +- runtime/hydradx/src/lib.rs | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 78678554f..733c20af1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4571,7 +4571,7 @@ dependencies = [ [[package]] name = "hydradx-adapters" -version = "1.1.1" +version = "1.1.2" dependencies = [ "cumulus-pallet-parachain-system", "cumulus-primitives-core", @@ -4619,7 +4619,7 @@ dependencies = [ [[package]] name = "hydradx-runtime" -version = "210.0.0" +version = "211.0.0" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", @@ -6980,7 +6980,7 @@ checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" [[package]] name = "pallet-asset-registry" -version = "3.0.0" +version = "3.0.1" dependencies = [ "frame-benchmarking", "frame-support", @@ -7398,7 +7398,7 @@ dependencies = [ [[package]] name = "pallet-dca" -version = "1.3.4" +version = "1.3.5" dependencies = [ "cumulus-pallet-parachain-system", "cumulus-primitives-core", @@ -8288,7 +8288,7 @@ dependencies = [ [[package]] name = "pallet-route-executor" -version = "2.0.1" +version = "2.0.2" dependencies = [ "frame-benchmarking", "frame-support", diff --git a/pallets/asset-registry/Cargo.toml b/pallets/asset-registry/Cargo.toml index 5d1d802ae..8d59d2733 100644 --- a/pallets/asset-registry/Cargo.toml +++ b/pallets/asset-registry/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-asset-registry" -version = "3.0.0" +version = "3.0.1" description = "Pallet for asset registry management" authors = ["GalacticCouncil"] edition = "2021" diff --git a/pallets/dca/Cargo.toml b/pallets/dca/Cargo.toml index 481420ca1..0afb0efdd 100644 --- a/pallets/dca/Cargo.toml +++ b/pallets/dca/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-dca' -version = "1.3.4" +version = "1.3.5" description = 'A pallet to manage DCA scheduling' authors = ['GalacticCouncil'] edition = '2021' diff --git a/pallets/route-executor/Cargo.toml b/pallets/route-executor/Cargo.toml index ca5b39e02..986f570db 100644 --- a/pallets/route-executor/Cargo.toml +++ b/pallets/route-executor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-route-executor' -version = '2.0.1' +version = '2.0.2' description = 'A pallet to execute a route containing a sequence of trades' authors = ['GalacticCouncil'] edition = '2021' diff --git a/runtime/adapters/Cargo.toml b/runtime/adapters/Cargo.toml index 33c5ad71a..2db927ab5 100644 --- a/runtime/adapters/Cargo.toml +++ b/runtime/adapters/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-adapters" -version = "1.1.1" +version = "1.1.2" description = "Structs and other generic types for building runtimes." authors = ["GalacticCouncil"] edition = "2021" diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index 9e6e872d6..e7f31a8dc 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "210.0.0" +version = "211.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index f06cd9325..7d71934f1 100644 --- a/runtime/hydradx/src/lib.rs +++ b/runtime/hydradx/src/lib.rs @@ -107,7 +107,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("hydradx"), impl_name: create_runtime_str!("hydradx"), authoring_version: 1, - spec_version: 210, + spec_version: 211, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From e4f2013700c8f9f80750ca5e9ad9340bc67aaf38 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 15:04:22 +0100 Subject: [PATCH 16/29] fix test involving stableswap --- integration-tests/src/router.rs | 67 ++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index 263b82411..c1d5421a9 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -749,48 +749,53 @@ mod omnipool_router_tests { .into()]); }); } + #[test] fn sell_should_work_when_user_has_left_less_than_existential_in_nonnative() { TestNet::reset(); Hydra::execute_with(|| { - //Arrange - let (pool_id, stable_asset_1, _) = init_stableswap().unwrap(); + let _ = with_transaction(|| { + //Arrange + let (pool_id, stable_asset_1, _) = init_stableswap().unwrap(); - init_omnipool(); + init_omnipool(); - let init_balance = 3000 * UNITS + 1; - assert_ok!(Currencies::update_balance( - hydradx_runtime::RuntimeOrigin::root(), - ALICE.into(), - stable_asset_1, - init_balance as i128, - )); + let init_balance = 3000 * UNITS + 1; + assert_ok!(Currencies::update_balance( + hydradx_runtime::RuntimeOrigin::root(), + ALICE.into(), + stable_asset_1, + init_balance as i128, + )); - let trades = vec![Trade { - pool: PoolType::Stableswap(pool_id), - asset_in: stable_asset_1, - asset_out: pool_id, - }]; + let trades = vec![Trade { + pool: PoolType::Stableswap(pool_id), + asset_in: stable_asset_1, + asset_out: pool_id, + }]; - assert_balance!(ALICE.into(), pool_id, 0); + assert_balance!(ALICE.into(), pool_id, 0); - //Act - let amount_to_sell = 3000 * UNITS; - assert_ok!(Router::sell( - hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), - stable_asset_1, - pool_id, - amount_to_sell, - 0, - trades - )); + //Act + let amount_to_sell = 3000 * UNITS; + assert_ok!(Router::sell( + hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), + stable_asset_1, + pool_id, + amount_to_sell, + 0, + trades + )); - //Assert - assert_eq!( - hydradx_runtime::Currencies::free_balance(stable_asset_1, &AccountId::from(ALICE)), - 0 - ); + //Assert + assert_eq!( + hydradx_runtime::Currencies::free_balance(stable_asset_1, &AccountId::from(ALICE)), + 0 + ); + + TransactionOutcome::Commit(DispatchResult::Ok(())) + }); }); } From 0112322840ad5a34969e03c29e98a2562a1a233b Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 17:11:43 +0100 Subject: [PATCH 17/29] remove balance check as it is problematic for insufficient assets so we discarded the it --- integration-tests/src/router.rs | 121 ++++++++++++++++++++++++ pallets/route-executor/src/lib.rs | 92 ------------------ pallets/route-executor/src/tests/buy.rs | 2 +- 3 files changed, 122 insertions(+), 93 deletions(-) diff --git a/integration-tests/src/router.rs b/integration-tests/src/router.rs index c1d5421a9..9e26ab7ee 100644 --- a/integration-tests/src/router.rs +++ b/integration-tests/src/router.rs @@ -707,6 +707,9 @@ mod router_different_pools_tests { mod omnipool_router_tests { use super::*; + use frame_support::assert_noop; + use hydradx_traits::router::PoolType; + use hydradx_traits::AssetKind; #[test] fn sell_should_work_when_route_contains_single_trade() { @@ -799,6 +802,124 @@ mod omnipool_router_tests { }); } + #[test] + fn sell_should_fail_when_all_asset_in_spent_for_shitcoin() { + TestNet::reset(); + + Hydra::execute_with(|| { + let _ = with_transaction(|| { + //Arrange + let name = b"SHITCO".to_vec(); + let shitcoin = AssetRegistry::register_insufficient_asset( + None, + Some(name.try_into().unwrap()), + AssetKind::External, + Some(1_000), + None, + None, + None, + None, + ) + .unwrap(); + assert_ok!(Currencies::deposit(shitcoin, &DAVE.into(), 100000 * UNITS,)); + assert_ok!(Currencies::update_balance( + hydradx_runtime::RuntimeOrigin::root(), + DAVE.into(), + HDX, + 100000 * UNITS as i128, + )); + + assert_ok!(XYK::create_pool( + RuntimeOrigin::signed(DAVE.into()), + HDX, + 100000 * UNITS, + shitcoin, + 100000 * UNITS, + )); + + let trades = vec![Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: shitcoin, + }]; + + //Act + let amount_to_sell = ALICE_INITIAL_NATIVE_BALANCE; + assert_noop!( + Router::sell( + hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), + HDX, + shitcoin, + amount_to_sell, + 0, + trades + ), + orml_tokens::Error::::ExistentialDeposit + ); + + TransactionOutcome::Commit(DispatchResult::Ok(())) + }); + }); + } + + #[test] + fn sell_should_pass_when_user_has_asset_in_covering_the_fee_for_shitcoin() { + TestNet::reset(); + + Hydra::execute_with(|| { + let _ = with_transaction(|| { + //Arrange + let name = b"SHITCO".to_vec(); + let shitcoin = AssetRegistry::register_insufficient_asset( + None, + Some(name.try_into().unwrap()), + AssetKind::External, + Some(1_000), + None, + None, + None, + None, + ) + .unwrap(); + + assert_ok!(Currencies::deposit(shitcoin, &DAVE.into(), 100000 * UNITS,)); + assert_ok!(Currencies::update_balance( + hydradx_runtime::RuntimeOrigin::root(), + DAVE.into(), + HDX, + 100000 * UNITS as i128, + )); + + assert_ok!(XYK::create_pool( + RuntimeOrigin::signed(DAVE.into()), + HDX, + 100000 * UNITS, + shitcoin, + 100000 * UNITS, + )); + + let trades = vec![Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: shitcoin, + }]; + + //Act + let amount_to_sell = ALICE_INITIAL_NATIVE_BALANCE - 20 * UNITS; + assert_ok!(Router::sell( + hydradx_runtime::RuntimeOrigin::signed(ALICE.into()), + HDX, + shitcoin, + amount_to_sell, + 0, + trades + )); + + TransactionOutcome::Commit(DispatchResult::Ok(())) + }); + }); + } + #[test] fn sell_should_work_when_user_has_left_less_than_existential_in_native() { TestNet::reset(); diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index eb6437f73..84fe76b85 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -133,12 +133,8 @@ pub mod pallet { MaxTradesExceeded, ///The AMM pool is not supported for executing trades PoolNotSupported, - /// Route has not trades to be executed - RouteHasNoTrades, ///The user has not enough balance to execute the trade InsufficientBalance, - ///The route execution failed in the underlying AMM - InvalidRouteExecution, ///The calculation of route trade amounts failed in the underlying AMM RouteCalculationFailed, ///The route is invalid @@ -194,8 +190,6 @@ pub mod pallet { let user_balance_of_asset_in_before_trade = T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); - let user_balance_of_asset_out_before_trade = - T::Currency::reducible_balance(asset_out, &who, Preservation::Expendable, Fortitude::Polite); ensure!( user_balance_of_asset_in_before_trade >= amount_in, @@ -211,11 +205,6 @@ pub mod pallet { ); for (trade_amount, trade) in trade_amounts.iter().zip(route) { - let user_balance_of_asset_in_before_trade = - T::Currency::reducible_balance(trade.asset_in, &who, Preservation::Expendable, Fortitude::Polite); - let existential_deposit = - T::ExistentialDepositGetter::get(&trade.asset_in).ok_or(Error::::NoExistentialDeposit)?; - let execution_result = T::AMM::execute_sell( origin.clone(), trade.pool, @@ -226,23 +215,8 @@ pub mod pallet { ); handle_execution_error!(execution_result); - - Self::ensure_that_user_spent_asset_in( - who.clone(), - trade.asset_in, - user_balance_of_asset_in_before_trade, - trade_amount.amount_in, - existential_deposit, - )?; } - Self::ensure_that_user_received_asset_out( - who, - asset_out, - user_balance_of_asset_out_before_trade, - last_trade_amount.amount_out, - )?; - Self::deposit_event(Event::Executed { asset_in, asset_out, @@ -277,26 +251,18 @@ pub mod pallet { max_amount_in: T::Balance, route: Vec>, ) -> DispatchResult { - let who = ensure_signed(origin.clone())?; Self::ensure_route_size(route.len())?; let asset_pair = AssetPair::new(asset_in, asset_out); let route = Self::get_route_or_default(route, asset_pair)?; Self::ensure_route_arguments(&asset_pair, &route)?; - let user_balance_of_asset_in_before_trade = - T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); - let existential_deposit = - T::ExistentialDepositGetter::get(&asset_in).ok_or(Error::::NoExistentialDeposit)?; let trade_amounts = Self::calculate_buy_trade_amounts(&route, amount_out)?; let first_trade = trade_amounts.last().ok_or(Error::::RouteCalculationFailed)?; ensure!(first_trade.amount_in <= max_amount_in, Error::::TradingLimitReached); for (trade_amount, trade) in trade_amounts.iter().rev().zip(route) { - let user_balance_of_asset_out_before_trade = - T::Currency::reducible_balance(trade.asset_out, &who, Preservation::Expendable, Fortitude::Polite); - let execution_result = T::AMM::execute_buy( origin.clone(), trade.pool, @@ -307,23 +273,8 @@ pub mod pallet { ); handle_execution_error!(execution_result); - - Self::ensure_that_user_received_asset_out( - who.clone(), - trade.asset_out, - user_balance_of_asset_out_before_trade, - trade_amount.amount_out, - )?; } - Self::ensure_that_user_spent_asset_in( - who, - asset_in, - user_balance_of_asset_in_before_trade, - first_trade.amount_in, - existential_deposit, - )?; - Self::deposit_event(Event::Executed { asset_in, asset_out, @@ -441,49 +392,6 @@ impl Pallet { Ok(()) } - fn ensure_that_user_received_asset_out( - who: T::AccountId, - asset_out: T::AssetId, - user_balance_of_asset_out_before_trade: T::Balance, - received_amount: T::Balance, - ) -> Result<(), DispatchError> { - let user_balance_of_asset_out_after_trade = - T::Currency::reducible_balance(asset_out, &who, Preservation::Expendable, Fortitude::Polite); - let user_expected_balance_of_asset_out_after_trade = user_balance_of_asset_out_before_trade - .checked_add(&received_amount) - .ok_or(ArithmeticError::Overflow)?; - - ensure!( - user_balance_of_asset_out_after_trade == user_expected_balance_of_asset_out_after_trade, - Error::::InvalidRouteExecution - ); - - Ok(()) - } - - fn ensure_that_user_spent_asset_in( - who: T::AccountId, - asset_in: T::AssetId, - user_balance_of_asset_in_before_trade: T::Balance, - spent_amount: T::Balance, - existential_deposit: T::Balance, - ) -> Result<(), DispatchError> { - let user_balance_of_asset_in_after_trade = - T::Currency::reducible_balance(asset_in, &who, Preservation::Expendable, Fortitude::Polite); - - let expected_user_balance_of_asset_in_after_trade = user_balance_of_asset_in_before_trade - .checked_sub(&spent_amount) - .ok_or(Error::::InvalidRouteExecution)?; - - //If the user had leftover less than ED then it is wiped out, hence we can't check the balance precisely - ensure!( - expected_user_balance_of_asset_in_after_trade < existential_deposit - || expected_user_balance_of_asset_in_after_trade == user_balance_of_asset_in_after_trade, - Error::::InvalidRouteExecution - ); - Ok(()) - } - fn get_route_or_default( route: Vec>, asset_pair: AssetPair, diff --git a/pallets/route-executor/src/tests/buy.rs b/pallets/route-executor/src/tests/buy.rs index 9fe547147..c4fb6afe6 100644 --- a/pallets/route-executor/src/tests/buy.rs +++ b/pallets/route-executor/src/tests/buy.rs @@ -465,7 +465,7 @@ fn buy_should_fail_when_called_with_non_signed_origin() { //Act and Assert assert_noop!( Router::buy(RuntimeOrigin::none(), HDX, AUSD, amount_to_buy, limit, trades), - BadOrigin + DispatchError::Other("Wrong origin") ); }); } From 7567012e3ce5e2354dfb51f4ade2d9573117e29a Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 18:09:46 +0100 Subject: [PATCH 18/29] disable setting onchain route for insufficient asset --- pallets/asset-registry/src/lib.rs | 10 ---- pallets/dca/src/tests/mock.rs | 37 ++++++++++++-- pallets/route-executor/src/lib.rs | 31 +++++++++--- pallets/route-executor/src/tests/mock.rs | 40 +++++++++++++-- pallets/route-executor/src/tests/set_route.rs | 50 +++++++++++++++++++ runtime/adapters/src/tests/mock.rs | 37 ++++++++++++-- runtime/hydradx/src/assets.rs | 2 +- 7 files changed, 173 insertions(+), 34 deletions(-) diff --git a/pallets/asset-registry/src/lib.rs b/pallets/asset-registry/src/lib.rs index dcb1bd16a..c91188006 100644 --- a/pallets/asset-registry/src/lib.rs +++ b/pallets/asset-registry/src/lib.rs @@ -638,16 +638,6 @@ impl GetByKey for Pallet { } } -/// Allows querying the ED for an asset by its id. -pub struct ExistentialDepositGetter(PhantomData); -/// Allows querying the ED for an asset by its id. -/// An unknown asset will return `None`. -impl GetByKey> for ExistentialDepositGetter { - fn get(k: &T::AssetId) -> Option { - Pallet::::assets(k).and_then(|details| Some(details.existential_deposit)) - } -} - /// Allows querying the XCM rate limit for an asset by its id. pub struct XcmRateLimitsInRegistry(PhantomData); /// Allows querying the XCM rate limit for an asset by its id. diff --git a/pallets/dca/src/tests/mock.rs b/pallets/dca/src/tests/mock.rs index bf237aa4f..e977ebfa8 100644 --- a/pallets/dca/src/tests/mock.rs +++ b/pallets/dca/src/tests/mock.rs @@ -349,11 +349,38 @@ parameter_types! { type Pools = (OmniPool, Xyk); -pub struct MockedExistentialDepositGetter; +pub struct MockedAssetRegistry; -impl GetByKey> for MockedExistentialDepositGetter { - fn get(_: &AssetId) -> Option { - Some(1000) +impl hydradx_traits::registry::Inspect for MockedAssetRegistry { + type AssetId = AssetId; + type Location = (); + + fn is_sufficient(_id: Self::AssetId) -> bool { + unimplemented!() + } + + fn exists(_id: Self::AssetId) -> bool { + unimplemented!() + } + + fn decimals(_id: Self::AssetId) -> Option { + unimplemented!() + } + + fn asset_type(_id: Self::AssetId) -> Option { + unimplemented!() + } + + fn is_banned(_id: Self::AssetId) -> bool { + unimplemented!() + } + + fn asset_name(_id: Self::AssetId) -> Option> { + unimplemented!() + } + + fn asset_symbol(_id: Self::AssetId) -> Option> { + unimplemented!() } } @@ -364,7 +391,7 @@ impl pallet_route_executor::Config for Test { type NativeAssetId = NativeCurrencyId; type Currency = FungibleCurrencies; type AMM = Pools; - type ExistentialDepositGetter = MockedExistentialDepositGetter; + type InspectRegistry = MockedAssetRegistry; type DefaultRoutePoolType = DefaultRoutePoolType; type WeightInfo = (); } diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index 84fe76b85..edb127abe 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -31,6 +31,7 @@ use frame_support::{ use frame_system::pallet_prelude::OriginFor; use frame_system::{ensure_signed, Origin}; +use hydradx_traits::registry::Inspect as RegistryInspect; use hydradx_traits::router::{inverse_route, AssetPair, RouteProvider}; pub use hydradx_traits::router::{ AmmTradeWeights, AmountInAndOut, ExecutorError, PoolType, RouterT, Trade, TradeExecution, @@ -60,6 +61,7 @@ pub mod pallet { use orml_traits::GetByKey; use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedDiv, Zero}; use sp_runtime::Saturating; + use sp_std::collections::btree_set::BTreeSet; #[pallet::pallet] pub struct Pallet(_); @@ -93,7 +95,7 @@ pub mod pallet { type Currency: Inspect + Mutate; - type ExistentialDepositGetter: GetByKey>; + type InspectRegistry: hydradx_traits::registry::Inspect; /// Handlers for AMM pools to calculate and execute trades type AMM: TradeExecution< @@ -141,6 +143,8 @@ pub mod pallet { InvalidRoute, ///The route update was not successful RouteUpdateIsNotSuccessful, + ///Insufficient asset is not supported for on chain routing + InsufficientAssetNotSupported, ///No existential deposit found for asset NoExistentialDeposit, } @@ -317,6 +321,7 @@ pub mod pallet { let _ = ensure_signed(origin.clone())?; Self::ensure_route_size(new_route.len())?; Self::ensure_route_arguments(&asset_pair, &new_route)?; + Self::ensure_route_has_no_insufficient_asset(&new_route)?; if !asset_pair.is_ordered() { asset_pair = asset_pair.ordered_pair(); @@ -392,6 +397,23 @@ impl Pallet { Ok(()) } + fn ensure_route_has_no_insufficient_asset(new_route: &Vec>) -> DispatchResult { + let mut unique_assets = sp_std::collections::btree_set::BTreeSet::new(); + + for trade in new_route.iter() { + unique_assets.insert(trade.asset_in.clone()); + unique_assets.insert(trade.asset_out.clone()); + } + for asset in unique_assets.iter() { + ensure!( + T::InspectRegistry::is_sufficient(*asset), + Error::::InsufficientAssetNotSupported + ); + } + + Ok(()) + } + fn get_route_or_default( route: Vec>, asset_pair: AssetPair, @@ -445,13 +467,6 @@ impl Pallet { with_transaction(|| { let origin: OriginFor = Origin::::Signed(Self::router_account()).into(); - //NOTE: This is necessary so router's account can pay ED for insufficient assets in the - //route. Value is 10K to make sure we can pay ED for really long routes. - let _ = T::Currency::mint_into( - T::NativeAssetId::get(), - &Self::router_account(), - 10_000_000_000_000_000_u128.into(), - ); let _ = T::Currency::mint_into(asset_in, &Self::router_account(), amount_in); let sell_result = Self::sell(origin, asset_in, asset_out, amount_in, u128::MIN.into(), route.clone()); diff --git a/pallets/route-executor/src/tests/mock.rs b/pallets/route-executor/src/tests/mock.rs index 25293c5e3..d3409874d 100644 --- a/pallets/route-executor/src/tests/mock.rs +++ b/pallets/route-executor/src/tests/mock.rs @@ -146,17 +146,45 @@ impl Config for Test { type Balance = Balance; type NativeAssetId = NativeCurrencyId; type Currency = FungibleCurrencies; - type ExistentialDepositGetter = MockedExistentialDepositGetter; + type InspectRegistry = MockedAssetRegistry; type AMM = Pools; type DefaultRoutePoolType = DefaultRoutePoolType; type WeightInfo = (); } -pub struct MockedExistentialDepositGetter; +use hydradx_traits::AssetKind; +pub struct MockedAssetRegistry; -impl GetByKey> for MockedExistentialDepositGetter { - fn get(_: &AssetId) -> Option { - Some(1000) +impl hydradx_traits::registry::Inspect for MockedAssetRegistry { + type AssetId = AssetId; + type Location = (); + + fn is_sufficient(id: Self::AssetId) -> bool { + id <= 2000 + } + + fn exists(_id: Self::AssetId) -> bool { + unimplemented!() + } + + fn decimals(_id: Self::AssetId) -> Option { + unimplemented!() + } + + fn asset_type(_id: Self::AssetId) -> Option { + unimplemented!() + } + + fn is_banned(_id: Self::AssetId) -> bool { + unimplemented!() + } + + fn asset_name(_id: Self::AssetId) -> Option> { + unimplemented!() + } + + fn asset_symbol(_id: Self::AssetId) -> Option> { + unimplemented!() } } @@ -173,6 +201,7 @@ pub const RMRK: AssetId = 1004; pub const SDN: AssetId = 1005; pub const STABLE_SHARE_ASSET: AssetId = 1006; pub const DOT: AssetId = 1007; +pub const INSUFFICIENT_ASSET: AssetId = 50000001; pub const ALICE_INITIAL_NATIVE_BALANCE: u128 = 1000; @@ -236,6 +265,7 @@ impl ExtBuilder { let mut initial_accounts = vec![ (ASSET_PAIR_ACCOUNT, STABLE_SHARE_ASSET, 1000u128), (ASSET_PAIR_ACCOUNT, AUSD, 1000u128), + (ASSET_PAIR_ACCOUNT, INSUFFICIENT_ASSET, 1000u128), (ASSET_PAIR_ACCOUNT, MOVR, 1000u128), (ASSET_PAIR_ACCOUNT, KSM, 1000u128), (ASSET_PAIR_ACCOUNT, RMRK, 1000u128), diff --git a/pallets/route-executor/src/tests/set_route.rs b/pallets/route-executor/src/tests/set_route.rs index 79d2603bb..545f9e45d 100644 --- a/pallets/route-executor/src/tests/set_route.rs +++ b/pallets/route-executor/src/tests/set_route.rs @@ -435,3 +435,53 @@ fn set_route_should_not_work_when_readded_the_same() { ); }); } + +#[test] +fn set_route_should_fail_with_insufficient_asset() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let asset_pair = AssetPair::new(HDX, INSUFFICIENT_ASSET); + let route = vec![Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: INSUFFICIENT_ASSET, + }]; + + //Act + assert_noop!( + Router::set_route(RuntimeOrigin::signed(ALICE), asset_pair, route.clone()), + Error::::InsufficientAssetNotSupported + ); + }); +} + +#[test] +fn set_route_should_fail_with_insufficient_asset_as_intermediare() { + ExtBuilder::default().build().execute_with(|| { + //Arrange + let asset_pair = AssetPair::new(HDX, DOT); + let route = vec![ + Trade { + pool: PoolType::XYK, + asset_in: HDX, + asset_out: AUSD, + }, + Trade { + pool: PoolType::XYK, + asset_in: AUSD, + asset_out: INSUFFICIENT_ASSET, + }, + Trade { + pool: PoolType::XYK, + asset_in: INSUFFICIENT_ASSET, + asset_out: DOT, + }, + ]; + + //Act + assert_noop!( + Router::set_route(RuntimeOrigin::signed(ALICE), asset_pair, route.clone()), + Error::::InsufficientAssetNotSupported + ); + }); +} diff --git a/runtime/adapters/src/tests/mock.rs b/runtime/adapters/src/tests/mock.rs index 2e953da95..aa2a222ea 100644 --- a/runtime/adapters/src/tests/mock.rs +++ b/runtime/adapters/src/tests/mock.rs @@ -318,11 +318,38 @@ parameter_types! { type Pools = (Omnipool, XYK); -pub struct MockedExistentialDepositGetter; +pub struct MockedAssetRegistry; -impl GetByKey> for MockedExistentialDepositGetter { - fn get(_: &AssetId) -> Option { - Some(1000) +impl hydradx_traits::registry::Inspect for MockedAssetRegistry { + type AssetId = AssetId; + type Location = (); + + fn is_sufficient(_id: Self::AssetId) -> bool { + true + } + + fn exists(_id: Self::AssetId) -> bool { + unimplemented!() + } + + fn decimals(_id: Self::AssetId) -> Option { + unimplemented!() + } + + fn asset_type(_id: Self::AssetId) -> Option { + unimplemented!() + } + + fn is_banned(_id: Self::AssetId) -> bool { + unimplemented!() + } + + fn asset_name(_id: Self::AssetId) -> Option> { + unimplemented!() + } + + fn asset_symbol(_id: Self::AssetId) -> Option> { + unimplemented!() } } impl pallet_route_executor::Config for Test { @@ -331,7 +358,7 @@ impl pallet_route_executor::Config for Test { type Balance = Balance; type NativeAssetId = NativeCurrencyId; type Currency = FungibleCurrencies; - type ExistentialDepositGetter = MockedExistentialDepositGetter; + type InspectRegistry = MockedAssetRegistry; type AMM = Pools; type DefaultRoutePoolType = DefaultRoutePoolType; type WeightInfo = (); diff --git a/runtime/hydradx/src/assets.rs b/runtime/hydradx/src/assets.rs index e6221ff3a..a3b1daefa 100644 --- a/runtime/hydradx/src/assets.rs +++ b/runtime/hydradx/src/assets.rs @@ -953,7 +953,7 @@ impl pallet_route_executor::Config for Runtime { type AMM = (Omnipool, Stableswap, XYK, LBP); type DefaultRoutePoolType = DefaultRoutePoolType; type NativeAssetId = NativeAssetId; - type ExistentialDepositGetter = pallet_asset_registry::ExistentialDepositGetter; + type InspectRegistry = AssetRegistry; } parameter_types! { From e2895749fc1e7fe9583c4d0a5e2ba65030e57943 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 18:14:20 +0100 Subject: [PATCH 19/29] make clippy happy --- pallets/route-executor/src/lib.rs | 10 ++++------ pallets/route-executor/src/tests/buy.rs | 2 +- pallets/route-executor/src/tests/mock.rs | 1 - pallets/route-executor/src/tests/set_route.rs | 4 ++-- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index edb127abe..c02923dd1 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -38,7 +38,7 @@ pub use hydradx_traits::router::{ }; use orml_traits::arithmetic::{CheckedAdd, CheckedSub}; use sp_runtime::traits::{AccountIdConversion, CheckedDiv}; -use sp_runtime::{ArithmeticError, DispatchError, Saturating, TransactionOutcome}; +use sp_runtime::{ArithmeticError, DispatchError, TransactionOutcome}; use sp_std::{vec, vec::Vec}; #[cfg(test)] @@ -58,10 +58,8 @@ pub mod pallet { use frame_support::traits::fungibles::Mutate; use frame_system::pallet_prelude::OriginFor; use hydradx_traits::router::ExecutorError; - use orml_traits::GetByKey; use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedDiv, Zero}; use sp_runtime::Saturating; - use sp_std::collections::btree_set::BTreeSet; #[pallet::pallet] pub struct Pallet(_); @@ -397,12 +395,12 @@ impl Pallet { Ok(()) } - fn ensure_route_has_no_insufficient_asset(new_route: &Vec>) -> DispatchResult { + fn ensure_route_has_no_insufficient_asset(new_route: &[Trade]) -> DispatchResult { let mut unique_assets = sp_std::collections::btree_set::BTreeSet::new(); for trade in new_route.iter() { - unique_assets.insert(trade.asset_in.clone()); - unique_assets.insert(trade.asset_out.clone()); + unique_assets.insert(trade.asset_in); + unique_assets.insert(trade.asset_out); } for asset in unique_assets.iter() { ensure!( diff --git a/pallets/route-executor/src/tests/buy.rs b/pallets/route-executor/src/tests/buy.rs index c4fb6afe6..475a4681a 100644 --- a/pallets/route-executor/src/tests/buy.rs +++ b/pallets/route-executor/src/tests/buy.rs @@ -22,7 +22,7 @@ use hydradx_traits::router::AssetPair; use hydradx_traits::router::PoolType; use pretty_assertions::assert_eq; use sp_runtime::DispatchError; -use sp_runtime::DispatchError::BadOrigin; + #[test] fn buy_should_work_when_route_has_single_trade() { ExtBuilder::default().build().execute_with(|| { diff --git a/pallets/route-executor/src/tests/mock.rs b/pallets/route-executor/src/tests/mock.rs index d3409874d..2db151dbb 100644 --- a/pallets/route-executor/src/tests/mock.rs +++ b/pallets/route-executor/src/tests/mock.rs @@ -24,7 +24,6 @@ use frame_support::{ use frame_system::{ensure_signed, pallet_prelude::OriginFor}; use hydradx_traits::router::{ExecutorError, PoolType, TradeExecution}; use orml_traits::parameter_type_with_key; -use orml_traits::GetByKey; use pallet_currencies::{fungibles::FungibleCurrencies, BasicCurrencyAdapter}; use pretty_assertions::assert_eq; use sp_core::H256; diff --git a/pallets/route-executor/src/tests/set_route.rs b/pallets/route-executor/src/tests/set_route.rs index 545f9e45d..3e7cc8e69 100644 --- a/pallets/route-executor/src/tests/set_route.rs +++ b/pallets/route-executor/src/tests/set_route.rs @@ -449,7 +449,7 @@ fn set_route_should_fail_with_insufficient_asset() { //Act assert_noop!( - Router::set_route(RuntimeOrigin::signed(ALICE), asset_pair, route.clone()), + Router::set_route(RuntimeOrigin::signed(ALICE), asset_pair, route), Error::::InsufficientAssetNotSupported ); }); @@ -480,7 +480,7 @@ fn set_route_should_fail_with_insufficient_asset_as_intermediare() { //Act assert_noop!( - Router::set_route(RuntimeOrigin::signed(ALICE), asset_pair, route.clone()), + Router::set_route(RuntimeOrigin::signed(ALICE), asset_pair, route), Error::::InsufficientAssetNotSupported ); }); From 02769d8fdcdd338eeeec735ed58b05ec717f0531 Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 18:17:58 +0100 Subject: [PATCH 20/29] remove unused error --- pallets/route-executor/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/pallets/route-executor/src/lib.rs b/pallets/route-executor/src/lib.rs index c02923dd1..cc6fdacc9 100644 --- a/pallets/route-executor/src/lib.rs +++ b/pallets/route-executor/src/lib.rs @@ -143,8 +143,6 @@ pub mod pallet { RouteUpdateIsNotSuccessful, ///Insufficient asset is not supported for on chain routing InsufficientAssetNotSupported, - ///No existential deposit found for asset - NoExistentialDeposit, } /// Storing routes for asset pairs From 6790d51e385b2a2b35e477d9e370ec415e198d5e Mon Sep 17 00:00:00 2001 From: dmoka Date: Wed, 21 Feb 2024 18:37:26 +0100 Subject: [PATCH 21/29] bump versions --- integration-tests/Cargo.toml | 2 +- runtime/hydradx/Cargo.toml | 2 +- runtime/hydradx/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 9a9e1f811..2116972e6 100644 --- a/integration-tests/Cargo.toml +++ b/integration-tests/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "runtime-integration-tests" -version = "1.18.1" +version = "1.18.2" description = "Integration tests" authors = ["GalacticCouncil"] edition = "2021" diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index e7f31a8dc..6d98783d9 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "211.0.0" +version = "212.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index de1957c81..91d059731 100644 --- a/runtime/hydradx/src/lib.rs +++ b/runtime/hydradx/src/lib.rs @@ -107,7 +107,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("hydradx"), impl_name: create_runtime_str!("hydradx"), authoring_version: 1, - spec_version: 211, + spec_version: 212, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From b1bbde044c3180d69fa5e90edf5658bb0008b0ba Mon Sep 17 00:00:00 2001 From: mrq Date: Thu, 22 Feb 2024 10:02:24 +0100 Subject: [PATCH 22/29] lock --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 93600cb1a..8ee4dd831 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11331,7 +11331,7 @@ dependencies = [ [[package]] name = "runtime-integration-tests" -version = "1.18.1" +version = "1.18.2" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", From f55a9255529dc934e282b2af35d0f9be738ee016 Mon Sep 17 00:00:00 2001 From: dmoka Date: Thu, 22 Feb 2024 11:57:56 +0100 Subject: [PATCH 23/29] wip - fix benchmarking --- runtime/hydradx/src/benchmarking/omnipool.rs | 2 +- .../src/benchmarking/route_executor.rs | 152 ++++++++++++++---- 2 files changed, 126 insertions(+), 28 deletions(-) diff --git a/runtime/hydradx/src/benchmarking/omnipool.rs b/runtime/hydradx/src/benchmarking/omnipool.rs index e1b43a0ae..d2eef93ef 100644 --- a/runtime/hydradx/src/benchmarking/omnipool.rs +++ b/runtime/hydradx/src/benchmarking/omnipool.rs @@ -47,7 +47,7 @@ fn run_to_block(to: u32) { const HDX: AssetId = 0; const DAI: AssetId = 2; -fn init() -> DispatchResult { +pub fn init() -> DispatchResult { let stable_amount: Balance = 1_000_000_000_000_000u128; let native_amount: Balance = 1_000_000_000_000_000u128; let stable_price: FixedU128 = FixedU128::from((1, 2)); diff --git a/runtime/hydradx/src/benchmarking/route_executor.rs b/runtime/hydradx/src/benchmarking/route_executor.rs index d26bfe169..499dd6efc 100644 --- a/runtime/hydradx/src/benchmarking/route_executor.rs +++ b/runtime/hydradx/src/benchmarking/route_executor.rs @@ -17,11 +17,13 @@ #![allow(clippy::result_large_err)] use crate::{ - AccountId, AssetId, Balance, Currencies, InsufficientEDinHDX, Router, Runtime, RuntimeOrigin, System, LBP, XYK, + AccountId, AssetId, Balance, Currencies, InsufficientEDinHDX, Omnipool, Router, Runtime, RuntimeOrigin, System, + LBP, XYK, }; use super::*; - +use crate::benchmarking::dca::HDX; +use crate::benchmarking::tokens::update_balance; use frame_benchmarking::{account, BenchmarkError}; use frame_support::dispatch::DispatchResult; use frame_support::{assert_ok, ensure}; @@ -31,8 +33,9 @@ use hydradx_traits::router::{PoolType, RouterT, Trade}; use orml_benchmarking::runtime_benchmarks; use orml_traits::{MultiCurrency, MultiCurrencyExtended}; use primitives::constants::currency::UNITS; +use sp_runtime::FixedU128; +use sp_runtime::Permill; use sp_std::vec; - pub const INITIAL_BALANCE: Balance = 10_000_000 * UNITS; fn funded_account(name: &'static str, index: u32, assets: &[AssetId]) -> AccountId { @@ -134,6 +137,39 @@ fn create_xyk_pool(asset_a: u32, asset_b: u32) { )); } +fn create_xyk_pool_with_amounts(asset_a: u32, asset_b: u32, amount_a: u128, amount_b: u128) { + let caller: AccountId = funded_account("caller", 3, &[asset_a, asset_b]); + + assert_ok!(Currencies::update_balance( + RawOrigin::Root.into(), + caller.clone(), + 0_u32, + InsufficientEDinHDX::get() as i128, + )); + + assert_ok!(Currencies::update_balance( + RuntimeOrigin::root(), + caller.clone(), + asset_a, + amount_a as i128, + )); + + assert_ok!(Currencies::update_balance( + RuntimeOrigin::root(), + caller.clone(), + asset_b, + amount_b as i128, + )); + + assert_ok!(XYK::create_pool( + RuntimeOrigin::signed(caller), + asset_a, + amount_a, + asset_b, + amount_b, + )); +} + runtime_benchmarks! { {Runtime, pallet_route_executor} @@ -208,48 +244,79 @@ runtime_benchmarks! { // Calculates the weight of xyk set route. Used in the calculation to determine the weight of the overhead. set_route_for_xyk { - let asset_1 = register_external_asset(b"FCA".to_vec()).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; - let asset_2 = register_external_asset(b"FCB".to_vec()).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; - let asset_3 = register_external_asset(b"FCC".to_vec()).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; + let asset_1 = register_asset(b"AS1".to_vec(), 1u128).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; + let asset_2 = register_asset(b"AS2".to_vec(), 1u128).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; + let asset_3 = register_asset(b"AS3".to_vec(), 1u128).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; + let asset_4 = register_asset(b"AS4".to_vec(), 1u128).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; + let asset_5 = register_asset(b"AS5".to_vec(), 1u128).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; + let asset_6 = register_asset(b"AS6".to_vec(), 1u128).map_err(|_| BenchmarkError::Stop("Failed to register asset"))?; + + let caller: AccountId = funded_account("caller", 0, &[asset_1, asset_2,asset_3]); + create_xyk_pool(HDX, asset_2); + create_xyk_pool(asset_2, asset_3); + create_xyk_pool(asset_3, asset_4); + create_xyk_pool(asset_4, asset_5); + create_xyk_pool(asset_5, asset_6); - let caller: AccountId = funded_account("caller", 0, &[asset_1, asset_2, asset_3]); - let buyer: AccountId = funded_account("buyer", 1, &[asset_1, asset_2, asset_3]); + //INIT OMNIPOOL + let acc = Omnipool::protocol_account(); + crate::benchmarking::omnipool::init()?; + // Create account for token provider and set balance + let owner: AccountId = account("owner", 0, 1); - create_xyk_pool(asset_1, asset_2); - create_xyk_pool(asset_1, asset_3); - create_xyk_pool(asset_2, asset_3); + let token_price = FixedU128::from((1,5)); + let token_amount = 200_000_000_000_000_000_u128; + //1000000000000000 + + update_balance(asset_6, &acc, token_amount); + + // Add the token to the pool + Omnipool::add_token(RawOrigin::Root.into(), asset_6, token_price, Permill::from_percent(100), owner)?; let route = vec![Trade { - pool: PoolType::XYK, - asset_in: asset_1, - asset_out: asset_2 - },Trade { - pool: PoolType::XYK, - asset_in: asset_2, - asset_out: asset_3 + pool: PoolType::Omnipool, + asset_in: HDX, + asset_out: asset_6 }]; Router::set_route( RawOrigin::Signed(caller.clone()).into(), - AssetPair::new(asset_1, asset_3), - route, + AssetPair::new(HDX, asset_6), + route.clone(), )?; + assert_eq!(3,4); + let better_route = vec![Trade { pool: PoolType::XYK, - asset_in: asset_1, + asset_in: HDX, + asset_out: asset_2 + },Trade { + pool: PoolType::XYK, + asset_in: asset_2, asset_out: asset_3 + },Trade { + pool: PoolType::XYK, + asset_in: asset_3, + asset_out: asset_4 + },Trade { + pool: PoolType::XYK, + asset_in: asset_4, + asset_out: asset_5 + },Trade { + pool: PoolType::XYK, + asset_in: asset_5, + asset_out: asset_6 }]; - }: { Router::set_route( RawOrigin::Signed(caller.clone()).into(), - AssetPair::new(asset_1, asset_3), + AssetPair::new(HDX, asset_6), better_route.clone(), )?; } verify { - let stored_route = Router::route(AssetPair::new(asset_1, asset_3)).unwrap(); + let stored_route = Router::route(AssetPair::new(HDX, asset_6)).unwrap(); assert_eq!(stored_route, better_route); } } @@ -257,14 +324,45 @@ runtime_benchmarks! { #[cfg(test)] mod tests { use super::*; + use crate::NativeExistentialDeposit; use orml_benchmarking::impl_benchmark_test_suite; use sp_runtime::BuildStorage; fn new_test_ext() -> sp_io::TestExternalities { - frame_system::GenesisConfig::::default() + let mut t = frame_system::GenesisConfig::::default() .build_storage() - .unwrap() - .into() + .unwrap(); + + pallet_asset_registry::GenesisConfig:: { + registered_assets: vec![ + ( + Some(1), + Some(b"LRNA".to_vec().try_into().unwrap()), + 1_000u128, + None, + None, + None, + true, + ), + ( + Some(2), + Some(b"DAI".to_vec().try_into().unwrap()), + 1_000u128, + None, + None, + None, + true, + ), + ], + native_asset_name: b"HDX".to_vec().try_into().unwrap(), + native_existential_deposit: NativeExistentialDeposit::get(), + native_decimals: 12, + native_symbol: b"HDX".to_vec().try_into().unwrap(), + } + .assimilate_storage(&mut t) + .unwrap(); + + sp_io::TestExternalities::new(t) } impl_benchmark_test_suite!(new_test_ext(),); From 09885fb9c56d97c8e09c507d38a3344332664964 Mon Sep 17 00:00:00 2001 From: dmoka Date: Thu, 22 Feb 2024 14:01:30 +0100 Subject: [PATCH 24/29] fix bencmhark tests --- .../hydradx/src/benchmarking/route_executor.rs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/runtime/hydradx/src/benchmarking/route_executor.rs b/runtime/hydradx/src/benchmarking/route_executor.rs index 499dd6efc..e197508be 100644 --- a/runtime/hydradx/src/benchmarking/route_executor.rs +++ b/runtime/hydradx/src/benchmarking/route_executor.rs @@ -264,8 +264,8 @@ runtime_benchmarks! { // Create account for token provider and set balance let owner: AccountId = account("owner", 0, 1); - let token_price = FixedU128::from((1,5)); - let token_amount = 200_000_000_000_000_000_u128; + let token_price = FixedU128::from((5,1)); + let token_amount = 100000 * UNITS; //1000000000000000 update_balance(asset_6, &acc, token_amount); @@ -273,20 +273,6 @@ runtime_benchmarks! { // Add the token to the pool Omnipool::add_token(RawOrigin::Root.into(), asset_6, token_price, Permill::from_percent(100), owner)?; - let route = vec![Trade { - pool: PoolType::Omnipool, - asset_in: HDX, - asset_out: asset_6 - }]; - - Router::set_route( - RawOrigin::Signed(caller.clone()).into(), - AssetPair::new(HDX, asset_6), - route.clone(), - )?; - - assert_eq!(3,4); - let better_route = vec![Trade { pool: PoolType::XYK, asset_in: HDX, From e0349e3d2e12cb6683afd0cea64db902cbf8c2d6 Mon Sep 17 00:00:00 2001 From: dmoka Date: Thu, 22 Feb 2024 14:24:20 +0100 Subject: [PATCH 25/29] rebenchmark pallet --- runtime/hydradx/src/weights/route_executor.rs | 112 +++++++++++------- 1 file changed, 68 insertions(+), 44 deletions(-) diff --git a/runtime/hydradx/src/weights/route_executor.rs b/runtime/hydradx/src/weights/route_executor.rs index b61909e67..5fbbf68a8 100644 --- a/runtime/hydradx/src/weights/route_executor.rs +++ b/runtime/hydradx/src/weights/route_executor.rs @@ -18,7 +18,7 @@ //! Autogenerated weights for `pallet_route_executor` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2024-02-15, STEPS: `10`, REPEAT: `30`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-02-22, STEPS: `10`, REPEAT: `30`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `bench-bot`, CPU: `Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: 1024 @@ -34,7 +34,7 @@ // --heap-pages=4096 // --template=.maintain/pallet-weight-template-no-back.hbs // --pallet=pallet-route-executor -// --output=./weights/route_executor.rs +// --output=router.rs // --extrinsic=* #![cfg_attr(rustfmt, rustfmt_skip)] @@ -45,11 +45,9 @@ use frame_support::{traits::Get, weights::Weight}; use core::marker::PhantomData; -use pallet_route_executor::weights::WeightInfo; - /// Weight functions for `pallet_route_executor`. pub struct HydraWeight(PhantomData); -impl WeightInfo for HydraWeight { +impl pallet_route_executor::WeightInfo for HydraWeight { /// Storage: `LBP::PoolData` (r:1 w:0) /// Proof: `LBP::PoolData` (`max_values`: None, `max_size`: Some(163), added: 2638, mode: `MaxEncodedLen`) /// Storage: `Tokens::Accounts` (r:5 w:5) @@ -69,10 +67,10 @@ impl WeightInfo for HydraWeight { // Proof Size summary in bytes: // Measured: `3402` // Estimated: `13905` - // Minimum execution time: 342_380_000 picoseconds. - Weight::from_parts(344_960_058, 13905) - // Standard Error: 141_835 - .saturating_add(Weight::from_parts(50_412_253, 0).saturating_mul(c.into())) + // Minimum execution time: 328_806_000 picoseconds. + Weight::from_parts(333_203_198, 13905) + // Standard Error: 371_747 + .saturating_add(Weight::from_parts(49_956_676, 0).saturating_mul(c.into())) .saturating_add(T::DbWeight::get().reads(16)) .saturating_add(T::DbWeight::get().writes(7)) } @@ -80,8 +78,6 @@ impl WeightInfo for HydraWeight { /// Proof: `LBP::PoolData` (`max_values`: None, `max_size`: Some(163), added: 2638, mode: `MaxEncodedLen`) /// Storage: `Tokens::Accounts` (r:5 w:5) /// Proof: `Tokens::Accounts` (`max_values`: None, `max_size`: Some(108), added: 2583, mode: `MaxEncodedLen`) - /// Storage: `System::Account` (r:3 w:1) - /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) /// Storage: `Tokens::Locks` (r:1 w:1) /// Proof: `Tokens::Locks` (`max_values`: None, `max_size`: Some(1261), added: 3736, mode: `MaxEncodedLen`) /// Storage: `Duster::AccountBlacklist` (r:2 w:0) @@ -90,58 +86,86 @@ impl WeightInfo for HydraWeight { /// Proof: `AssetRegistry::BannedAssets` (`max_values`: None, `max_size`: Some(20), added: 2495, mode: `MaxEncodedLen`) /// Storage: `AssetRegistry::Assets` (r:2 w:0) /// Proof: `AssetRegistry::Assets` (`max_values`: None, `max_size`: Some(125), added: 2600, mode: `MaxEncodedLen`) + /// Storage: `System::Account` (r:3 w:1) + /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) /// The range of component `c` is `[1, 2]`. /// The range of component `b` is `[0, 1]`. fn calculate_and_execute_buy_in_lbp(c: u32, b: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `1571 + b * (1836 ±0)` - // Estimated: `6156 + b * (7749 ±245_709_589_663_843_264)` - // Minimum execution time: 75_535_000 picoseconds. - Weight::from_parts(76_397_000, 6156) - // Standard Error: 600_512 - .saturating_add(Weight::from_parts(2_328_934, 0).saturating_mul(c.into())) - // Standard Error: 1_318_297 - .saturating_add(Weight::from_parts(272_259_348, 0).saturating_mul(b.into())) + // Estimated: `6156 + b * (7749 ±99_524_913_928_918_768)` + // Minimum execution time: 75_691_000 picoseconds. + Weight::from_parts(76_206_000, 6156) + // Standard Error: 581_036 + .saturating_add(Weight::from_parts(2_286_269, 0).saturating_mul(c.into())) + // Standard Error: 1_275_540 + .saturating_add(Weight::from_parts(259_214_464, 0).saturating_mul(b.into())) .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().reads((13_u64).saturating_mul(b.into()))) .saturating_add(T::DbWeight::get().writes((7_u64).saturating_mul(b.into()))) .saturating_add(Weight::from_parts(0, 7749).saturating_mul(b.into())) } + /// Storage: `AssetRegistry::Assets` (r:6 w:0) + /// Proof: `AssetRegistry::Assets` (`max_values`: None, `max_size`: Some(125), added: 2600, mode: `MaxEncodedLen`) /// Storage: `Router::Routes` (r:1 w:1) /// Proof: `Router::Routes` (`max_values`: None, `max_size`: Some(90), added: 2565, mode: `MaxEncodedLen`) - /// Storage: `Tokens::Accounts` (r:9 w:0) - /// Proof: `Tokens::Accounts` (`max_values`: None, `max_size`: Some(108), added: 2583, mode: `MaxEncodedLen`) - /// Storage: `System::Account` (r:5 w:0) + /// Storage: `Omnipool::Assets` (r:2 w:0) + /// Proof: `Omnipool::Assets` (`max_values`: None, `max_size`: Some(85), added: 2560, mode: `MaxEncodedLen`) + /// Storage: `System::Account` (r:8 w:0) /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) - /// Storage: `Tokens::TotalIssuance` (r:2 w:0) - /// Proof: `Tokens::TotalIssuance` (`max_values`: None, `max_size`: Some(28), added: 2503, mode: `MaxEncodedLen`) - /// Storage: `AssetRegistry::Assets` (r:3 w:0) - /// Proof: `AssetRegistry::Assets` (`max_values`: None, `max_size`: Some(125), added: 2600, mode: `MaxEncodedLen`) - /// Storage: `AssetRegistry::BannedAssets` (r:3 w:0) + /// Storage: `Tokens::Accounts` (r:16 w:0) + /// Proof: `Tokens::Accounts` (`max_values`: None, `max_size`: Some(108), added: 2583, mode: `MaxEncodedLen`) + /// Storage: `Omnipool::HubAssetImbalance` (r:1 w:0) + /// Proof: `Omnipool::HubAssetImbalance` (`max_values`: Some(1), `max_size`: Some(17), added: 512, mode: `MaxEncodedLen`) + /// Storage: `DynamicFees::AssetFee` (r:2 w:0) + /// Proof: `DynamicFees::AssetFee` (`max_values`: None, `max_size`: Some(24), added: 2499, mode: `MaxEncodedLen`) + /// Storage: `EmaOracle::Oracles` (r:2 w:0) + /// Proof: `EmaOracle::Oracles` (`max_values`: None, `max_size`: Some(177), added: 2652, mode: `MaxEncodedLen`) + /// Storage: `Duster::AccountBlacklist` (r:7 w:0) + /// Proof: `Duster::AccountBlacklist` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `AssetRegistry::BannedAssets` (r:5 w:0) /// Proof: `AssetRegistry::BannedAssets` (`max_values`: None, `max_size`: Some(20), added: 2495, mode: `MaxEncodedLen`) - /// Storage: `MultiTransactionPayment::AccountCurrencyMap` (r:1 w:0) + /// Storage: `MultiTransactionPayment::AccountCurrencyMap` (r:2 w:0) /// Proof: `MultiTransactionPayment::AccountCurrencyMap` (`max_values`: None, `max_size`: Some(52), added: 2527, mode: `MaxEncodedLen`) - /// Storage: `Balances::Locks` (r:1 w:0) - /// Proof: `Balances::Locks` (`max_values`: None, `max_size`: Some(1299), added: 3774, mode: `MaxEncodedLen`) - /// Storage: `Balances::Freezes` (r:1 w:0) - /// Proof: `Balances::Freezes` (`max_values`: None, `max_size`: Some(49), added: 2524, mode: `MaxEncodedLen`) - /// Storage: `AssetRegistry::ExistentialDepositCounter` (r:1 w:0) - /// Proof: `AssetRegistry::ExistentialDepositCounter` (`max_values`: Some(1), `max_size`: Some(16), added: 511, mode: `MaxEncodedLen`) - /// Storage: `MultiTransactionPayment::AcceptedCurrencies` (r:3 w:0) + /// Storage: `MultiTransactionPayment::AcceptedCurrencies` (r:5 w:0) /// Proof: `MultiTransactionPayment::AcceptedCurrencies` (`max_values`: None, `max_size`: Some(28), added: 2503, mode: `MaxEncodedLen`) - /// Storage: `XYK::ShareToken` (r:3 w:0) - /// Proof: `XYK::ShareToken` (`max_values`: None, `max_size`: Some(52), added: 2527, mode: `MaxEncodedLen`) - /// Storage: `Duster::AccountBlacklist` (r:4 w:0) - /// Proof: `Duster::AccountBlacklist` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) /// Storage: `EmaOracle::Accumulator` (r:1 w:0) /// Proof: `EmaOracle::Accumulator` (`max_values`: Some(1), `max_size`: Some(5921), added: 6416, mode: `MaxEncodedLen`) + /// Storage: `CircuitBreaker::AllowedTradeVolumeLimitPerAsset` (r:2 w:0) + /// Proof: `CircuitBreaker::AllowedTradeVolumeLimitPerAsset` (`max_values`: None, `max_size`: Some(68), added: 2543, mode: `MaxEncodedLen`) + /// Storage: `CircuitBreaker::TradeVolumeLimitPerAsset` (r:2 w:0) + /// Proof: `CircuitBreaker::TradeVolumeLimitPerAsset` (`max_values`: None, `max_size`: Some(28), added: 2503, mode: `MaxEncodedLen`) + /// Storage: `CircuitBreaker::LiquidityAddLimitPerAsset` (r:1 w:0) + /// Proof: `CircuitBreaker::LiquidityAddLimitPerAsset` (`max_values`: None, `max_size`: Some(29), added: 2504, mode: `MaxEncodedLen`) + /// Storage: `CircuitBreaker::AllowedAddLiquidityAmountPerAsset` (r:1 w:0) + /// Proof: `CircuitBreaker::AllowedAddLiquidityAmountPerAsset` (`max_values`: None, `max_size`: Some(52), added: 2527, mode: `MaxEncodedLen`) + /// Storage: `CircuitBreaker::LiquidityRemoveLimitPerAsset` (r:1 w:0) + /// Proof: `CircuitBreaker::LiquidityRemoveLimitPerAsset` (`max_values`: None, `max_size`: Some(29), added: 2504, mode: `MaxEncodedLen`) + /// Storage: `CircuitBreaker::AllowedRemoveLiquidityAmountPerAsset` (r:1 w:0) + /// Proof: `CircuitBreaker::AllowedRemoveLiquidityAmountPerAsset` (`max_values`: None, `max_size`: Some(52), added: 2527, mode: `MaxEncodedLen`) + /// Storage: `Referrals::LinkedAccounts` (r:1 w:0) + /// Proof: `Referrals::LinkedAccounts` (`max_values`: None, `max_size`: Some(80), added: 2555, mode: `MaxEncodedLen`) + /// Storage: `Referrals::AssetRewards` (r:1 w:0) + /// Proof: `Referrals::AssetRewards` (`max_values`: None, `max_size`: Some(49), added: 2524, mode: `MaxEncodedLen`) + /// Storage: `Referrals::TotalShares` (r:1 w:0) + /// Proof: `Referrals::TotalShares` (`max_values`: Some(1), `max_size`: Some(16), added: 511, mode: `MaxEncodedLen`) + /// Storage: `Referrals::TraderShares` (r:1 w:0) + /// Proof: `Referrals::TraderShares` (`max_values`: None, `max_size`: Some(64), added: 2539, mode: `MaxEncodedLen`) + /// Storage: `Referrals::PendingConversions` (r:1 w:0) + /// Proof: `Referrals::PendingConversions` (`max_values`: None, `max_size`: Some(20), added: 2495, mode: `MaxEncodedLen`) + /// Storage: `Referrals::CounterForPendingConversions` (r:1 w:0) + /// Proof: `Referrals::CounterForPendingConversions` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Tokens::TotalIssuance` (r:1 w:0) + /// Proof: `Tokens::TotalIssuance` (`max_values`: None, `max_size`: Some(28), added: 2503, mode: `MaxEncodedLen`) + /// Storage: `XYK::ShareToken` (r:5 w:0) + /// Proof: `XYK::ShareToken` (`max_values`: None, `max_size`: Some(52), added: 2527, mode: `MaxEncodedLen`) fn set_route_for_xyk() -> Weight { // Proof Size summary in bytes: - // Measured: `4957` - // Estimated: `24237` - // Minimum execution time: 2_485_264_000 picoseconds. - Weight::from_parts(2_495_249_000, 24237) - .saturating_add(T::DbWeight::get().reads(38)) + // Measured: `7137` + // Estimated: `42318` + // Minimum execution time: 2_156_649_000 picoseconds. + Weight::from_parts(2_172_125_000, 42318) + .saturating_add(T::DbWeight::get().reads(78)) .saturating_add(T::DbWeight::get().writes(1)) } -} +} \ No newline at end of file From 10ac36b55dd1c71bb5c390fc1d127fb152d9ba4b Mon Sep 17 00:00:00 2001 From: dmoka Date: Thu, 22 Feb 2024 14:27:11 +0100 Subject: [PATCH 26/29] remove unsed method --- .../src/benchmarking/route_executor.rs | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/runtime/hydradx/src/benchmarking/route_executor.rs b/runtime/hydradx/src/benchmarking/route_executor.rs index e197508be..cdba43295 100644 --- a/runtime/hydradx/src/benchmarking/route_executor.rs +++ b/runtime/hydradx/src/benchmarking/route_executor.rs @@ -137,39 +137,6 @@ fn create_xyk_pool(asset_a: u32, asset_b: u32) { )); } -fn create_xyk_pool_with_amounts(asset_a: u32, asset_b: u32, amount_a: u128, amount_b: u128) { - let caller: AccountId = funded_account("caller", 3, &[asset_a, asset_b]); - - assert_ok!(Currencies::update_balance( - RawOrigin::Root.into(), - caller.clone(), - 0_u32, - InsufficientEDinHDX::get() as i128, - )); - - assert_ok!(Currencies::update_balance( - RuntimeOrigin::root(), - caller.clone(), - asset_a, - amount_a as i128, - )); - - assert_ok!(Currencies::update_balance( - RuntimeOrigin::root(), - caller.clone(), - asset_b, - amount_b as i128, - )); - - assert_ok!(XYK::create_pool( - RuntimeOrigin::signed(caller), - asset_a, - amount_a, - asset_b, - amount_b, - )); -} - runtime_benchmarks! { {Runtime, pallet_route_executor} From 44cced4e1be9047e6692303014206e3e27088f14 Mon Sep 17 00:00:00 2001 From: dmoka Date: Thu, 22 Feb 2024 14:31:39 +0100 Subject: [PATCH 27/29] bump runtime --- Cargo.lock | 12 ++++++------ runtime/hydradx/Cargo.toml | 2 +- runtime/hydradx/src/lib.rs | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4d5e11747..166d126bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4571,7 +4571,7 @@ dependencies = [ [[package]] name = "hydradx-adapters" -version = "1.2.0" +version = "1.2.1" dependencies = [ "cumulus-pallet-parachain-system", "cumulus-primitives-core", @@ -4621,7 +4621,7 @@ dependencies = [ [[package]] name = "hydradx-runtime" -version = "213.0.0" +version = "214.0.0" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", @@ -6982,7 +6982,7 @@ checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" [[package]] name = "pallet-asset-registry" -version = "3.0.0" +version = "3.0.1" dependencies = [ "frame-benchmarking", "frame-support", @@ -7400,7 +7400,7 @@ dependencies = [ [[package]] name = "pallet-dca" -version = "1.3.4" +version = "1.3.5" dependencies = [ "cumulus-pallet-parachain-system", "cumulus-primitives-core", @@ -8290,7 +8290,7 @@ dependencies = [ [[package]] name = "pallet-route-executor" -version = "2.0.1" +version = "2.0.2" dependencies = [ "frame-benchmarking", "frame-support", @@ -11333,7 +11333,7 @@ dependencies = [ [[package]] name = "runtime-integration-tests" -version = "1.19.0" +version = "1.19.1" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-dmp-queue", diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index 60179b704..e3246eed7 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "213.0.0" +version = "214.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index eac6d9a40..a40b4868a 100644 --- a/runtime/hydradx/src/lib.rs +++ b/runtime/hydradx/src/lib.rs @@ -107,7 +107,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("hydradx"), impl_name: create_runtime_str!("hydradx"), authoring_version: 1, - spec_version: 213, + spec_version: 214, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From f91af68e376760f65d7241a1f44d0792ec4b4390 Mon Sep 17 00:00:00 2001 From: dmoka Date: Thu, 22 Feb 2024 14:36:41 +0100 Subject: [PATCH 28/29] fix compilation error --- runtime/hydradx/src/weights/route_executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/hydradx/src/weights/route_executor.rs b/runtime/hydradx/src/weights/route_executor.rs index 5fbbf68a8..d5a1bffcb 100644 --- a/runtime/hydradx/src/weights/route_executor.rs +++ b/runtime/hydradx/src/weights/route_executor.rs @@ -47,7 +47,7 @@ use core::marker::PhantomData; /// Weight functions for `pallet_route_executor`. pub struct HydraWeight(PhantomData); -impl pallet_route_executor::WeightInfo for HydraWeight { +impl pallet_route_executor::weights::WeightInfo for HydraWeight { /// Storage: `LBP::PoolData` (r:1 w:0) /// Proof: `LBP::PoolData` (`max_values`: None, `max_size`: Some(163), added: 2638, mode: `MaxEncodedLen`) /// Storage: `Tokens::Accounts` (r:5 w:5) From 3ad36867ea038cdfe2694a3a2cf1e9eba41d6b5c Mon Sep 17 00:00:00 2001 From: dmoka Date: Thu, 22 Feb 2024 14:36:47 +0100 Subject: [PATCH 29/29] bump patch version --- Cargo.lock | 2 +- pallets/asset-registry/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 166d126bf..49c701cbc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6982,7 +6982,7 @@ checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" [[package]] name = "pallet-asset-registry" -version = "3.0.1" +version = "3.1.0" dependencies = [ "frame-benchmarking", "frame-support", diff --git a/pallets/asset-registry/Cargo.toml b/pallets/asset-registry/Cargo.toml index 8d59d2733..6932b1565 100644 --- a/pallets/asset-registry/Cargo.toml +++ b/pallets/asset-registry/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-asset-registry" -version = "3.0.1" +version = "3.1.0" description = "Pallet for asset registry management" authors = ["GalacticCouncil"] edition = "2021"