From f0e4412cc6358c60247ba40f08e83a8b96f13aa6 Mon Sep 17 00:00:00 2001 From: martinfridrich Date: Tue, 14 Nov 2023 15:37:54 +0100 Subject: [PATCH 1/2] fix PR comments --- .../src/insufficient_assets_ed.rs | 56 +++++++++---------- pallets/asset-registry/src/benchmarking.rs | 6 +- pallets/asset-registry/src/lib.rs | 55 ++++++++++++------ pallets/asset-registry/src/tests/register.rs | 13 +++-- pallets/xyk/src/benchmarking.rs | 2 +- runtime/hydradx/src/assets.rs | 2 +- .../src/benchmarking/route_executor.rs | 3 +- 7 files changed, 80 insertions(+), 57 deletions(-) diff --git a/integration-tests/src/insufficient_assets_ed.rs b/integration-tests/src/insufficient_assets_ed.rs index 9eb574d6f..51ab7224e 100644 --- a/integration-tests/src/insufficient_assets_ed.rs +++ b/integration-tests/src/insufficient_assets_ed.rs @@ -20,7 +20,7 @@ use xcm_emulator::TestExt; fn sender_should_pay_ed_in_hdx_when_it_is_not_whitelisted() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); assert_ok!(Tokens::set_balance( RawOrigin::Root.into(), BOB.into(), @@ -79,7 +79,7 @@ fn sender_should_pay_ed_in_hdx_when_it_is_not_whitelisted() { fn reciever_should_pay_ed_in_hdx_when_insuficcient_asset_was_depositted() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); assert_ok!(Tokens::set_balance( RawOrigin::Root.into(), BOB.into(), @@ -131,7 +131,7 @@ fn reciever_should_pay_ed_in_hdx_when_insuficcient_asset_was_depositted() { fn hdx_ed_should_be_released_when_account_is_killed_and_ed_was_paid_in_hdx() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); assert_ok!(Tokens::set_balance( RawOrigin::Root.into(), BOB.into(), @@ -189,7 +189,7 @@ fn hdx_ed_should_be_released_when_account_is_killed_and_ed_was_paid_in_hdx() { fn sender_should_pay_ed_only_when_dest_didnt_pay_yet() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); let fee_asset = BTC; assert_ok!(Tokens::set_balance( @@ -273,7 +273,7 @@ fn sender_should_pay_ed_only_when_dest_didnt_pay_yet() { fn dest_should_pay_ed_only_once_when_insufficient_asset_was_depsitted() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); let fee_asset = BTC; assert_ok!(Tokens::set_balance( @@ -341,7 +341,7 @@ fn dest_should_pay_ed_only_once_when_insufficient_asset_was_depsitted() { fn hdx_ed_should_be_released_when_account_is_killed_and_ed_was_paid_in_fee_asset() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); let fee_asset = BTC; //NOTE: this is important for this tests - it basically mean that Bob already paid ED. @@ -425,7 +425,7 @@ fn hdx_ed_should_be_released_when_account_is_killed_and_ed_was_paid_in_fee_asset fn tx_should_fail_with_existential_deposit_err_when_dest_account_cant_pay_ed() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); let fee_asset = BTC; assert_ok!(MultiTransactionPayment::set_currency( @@ -449,7 +449,7 @@ fn tx_should_fail_with_existential_deposit_err_when_dest_account_cant_pay_ed() { fn sender_should_pay_ed_in_fee_asset_when_sending_insufficient_asset() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); let fee_asset = BTC; assert_ok!(Tokens::set_balance( @@ -591,7 +591,7 @@ fn grandfathered_account_should_receive_hdx_when_account_is_killed() { fn ed_should_not_be_collected_when_transfering_or_depositing_sufficient_assets() { TestNet::reset(); Hydra::execute_with(|| { - let sht1 = register_shitcoin(0_u128); + let sht1 = register_external_asset(0_u128); let sufficient_asset = DAI; //This pays ED. @@ -672,7 +672,7 @@ fn ed_should_not_be_collected_when_transfering_or_depositing_sufficient_assets() fn ed_should_not_be_released_when_sufficient_asset_killed_account() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); let sufficient_asset = DAI; //This pays ED. @@ -735,10 +735,10 @@ fn ed_should_not_be_released_when_sufficient_asset_killed_account() { fn ed_should_be_collected_for_each_insufficient_asset_when_transfered_or_depositted() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); - let sht2: AssetId = register_shitcoin(1_u128); - let sht3: AssetId = register_shitcoin(2_u128); - let sht4: AssetId = register_shitcoin(3_u128); + let sht1: AssetId = register_external_asset(0_u128); + let sht2: AssetId = register_external_asset(1_u128); + let sht3: AssetId = register_external_asset(2_u128); + let sht4: AssetId = register_external_asset(3_u128); let alice_hdx_balance = Currencies::free_balance(HDX, &ALICE.into()); let bob_hdx_balance = Currencies::free_balance(HDX, &BOB.into()); @@ -841,10 +841,10 @@ fn ed_should_be_collected_for_each_insufficient_asset_when_transfered_or_deposit fn ed_should_be_released_for_each_insufficient_asset_when_account_is_killed() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); - let sht2: AssetId = register_shitcoin(1_u128); - let sht3: AssetId = register_shitcoin(2_u128); - let sht4: AssetId = register_shitcoin(3_u128); + let sht1: AssetId = register_external_asset(0_u128); + let sht2: AssetId = register_external_asset(1_u128); + let sht3: AssetId = register_external_asset(2_u128); + let sht4: AssetId = register_external_asset(3_u128); //so bob doesn't pay ed assert_ok!(Tokens::set_balance(RawOrigin::Root.into(), BOB.into(), sht1, 1, 0)); @@ -968,10 +968,10 @@ fn ed_should_be_released_for_each_insufficient_asset_when_account_is_killed() { fn mix_of_sufficinet_and_insufficient_assets_should_lock_unlock_ed_correctly() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); - let sht2: AssetId = register_shitcoin(1_u128); - let sht3: AssetId = register_shitcoin(2_u128); - let sht4: AssetId = register_shitcoin(3_u128); + let sht1: AssetId = register_external_asset(0_u128); + let sht2: AssetId = register_external_asset(1_u128); + let sht3: AssetId = register_external_asset(2_u128); + let sht4: AssetId = register_external_asset(3_u128); //so bob doesn't pay ed assert_ok!(Tokens::set_balance(RawOrigin::Root.into(), BOB.into(), sht1, 1, 0)); @@ -1143,8 +1143,8 @@ fn mix_of_sufficinet_and_insufficient_assets_should_lock_unlock_ed_correctly() { fn sender_should_pay_ed_when_tranferred_or_deposited_to_whitelisted_dest() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); - let sht2: AssetId = register_shitcoin(1_u128); + let sht1: AssetId = register_external_asset(0_u128); + let sht2: AssetId = register_external_asset(1_u128); assert_ok!(Tokens::set_balance( RawOrigin::Root.into(), @@ -1226,7 +1226,7 @@ fn sender_should_pay_ed_when_tranferred_or_deposited_to_whitelisted_dest() { fn ed_should_be_released_when_whitelisted_account_was_killed() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); + let sht1: AssetId = register_external_asset(0_u128); let treasury = TreasuryAccount::get(); assert_ok!(Tokens::set_balance( @@ -1292,8 +1292,8 @@ fn ed_should_be_released_when_whitelisted_account_was_killed() { fn tx_should_fail_with_unsupported_currency_error_when_fee_asset_price_wasn_not_provided() { TestNet::reset(); Hydra::execute_with(|| { - let sht1: AssetId = register_shitcoin(0_u128); - let sht2: AssetId = register_shitcoin(1_u128); + let sht1: AssetId = register_external_asset(0_u128); + let sht2: AssetId = register_external_asset(1_u128); let fee_asset = BTC; assert_ok!(Tokens::set_balance( @@ -1335,7 +1335,7 @@ fn tx_should_fail_with_unsupported_currency_error_when_fee_asset_price_wasn_not_ }); } -fn register_shitcoin(general_index: u128) -> AssetId { +fn register_external_asset(general_index: u128) -> AssetId { let location = hydradx_runtime::AssetLocation(MultiLocation::new( 1, X2(Parachain(MOONBEAM_PARA_ID), GeneralIndex(general_index)), diff --git a/pallets/asset-registry/src/benchmarking.rs b/pallets/asset-registry/src/benchmarking.rs index 95804aea1..8f9f650e5 100644 --- a/pallets/asset-registry/src/benchmarking.rs +++ b/pallets/asset-registry/src/benchmarking.rs @@ -21,15 +21,15 @@ use super::*; use crate::types::AssetDetails; use frame_benchmarking::{account, benchmarks}; +use frame_support::traits::tokens::fungibles::Mutate as FungiblesMutate; use frame_system::RawOrigin; -use orml_traits::MultiCurrencyExtended; use sp_std::vec; const UNIT: u128 = 1_000_000_000_000; benchmarks! { where_clause { where - T::Currency: MultiCurrencyExtended, + T::Currency: FungiblesMutate, T: crate::pallet::Config, } @@ -92,7 +92,7 @@ benchmarks! { register_external { let caller: T::AccountId = account("caller", 0, 1); - T::Currency::update_balance(T::StorageFeesAssetId::get(), &caller, (100_000 * UNIT) as i128)?; + T::Currency::mint_into(T::StorageFeesAssetId::get(), &caller, 101_000 * UNIT)?; let expected_asset_id = Pallet::::next_asset_id().unwrap(); let location: T::AssetNativeLocation = Default::default(); diff --git a/pallets/asset-registry/src/lib.rs b/pallets/asset-registry/src/lib.rs index be34a5af6..57a1ca6d5 100644 --- a/pallets/asset-registry/src/lib.rs +++ b/pallets/asset-registry/src/lib.rs @@ -19,9 +19,9 @@ use frame_support::pallet_prelude::*; use frame_support::sp_runtime::traits::CheckedAdd; +use frame_support::traits::tokens::fungibles::{Inspect as FungiblesInspect, Transfer}; use frame_support::{dispatch::DispatchError, require_transactional}; use frame_system::pallet_prelude::*; -use orml_traits::MultiCurrency; use scale_info::TypeInfo; use sp_arithmetic::traits::{BaseArithmetic, Zero}; use sp_std::convert::TryInto; @@ -88,7 +88,8 @@ pub mod pallet { type AssetNativeLocation: Parameter + Member + Default + MaxEncodedLen; /// Multi currency mechanism - type Currency: MultiCurrency; + type Currency: FungiblesInspect + + Transfer; #[pallet::constant] type SequentialIdStartAt: Get; @@ -157,6 +158,23 @@ pub mod pallet { //NOTE: This error is triggered from `SufficiencyCheck`. /// Existential deposit can't be zero. ZeroExistentialDeposit, + + /// Action cannot be completed because unexpected error has occurred. This should be reported + /// to protocol maintainers. + InconsistentState(InconsistentStateError), + } + + // NOTE: these errors should never happen. + #[derive(Encode, Decode, Eq, PartialEq, TypeInfo, frame_support::PalletError, RuntimeDebug)] + pub enum InconsistentStateError { + /// Name or symbol conversion to bounded string failed. + BoundedConversionFailed, + } + + impl From for Error { + fn from(e: InconsistentStateError) -> Error { + Error::::InconsistentState(e) + } } #[pallet::type_value] @@ -232,7 +250,7 @@ pub mod pallet { #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - let _ = with_transaction(|| { + with_transaction(|| { // Register native asset first // It is to make sure that native is registered as any other asset let native_asset_name = Pallet::::try_into_bounded(Some(self.native_asset_name.to_vec())) @@ -285,7 +303,8 @@ pub mod pallet { ); TransactionOutcome::Commit(DispatchResult::Ok(())) - }); + }) + .expect("Genesis build failed.") } } @@ -475,7 +494,9 @@ pub mod pallet { if !T::StorageFees::get().is_zero() { ensure!( - T::Currency::ensure_can_withdraw(T::StorageFeesAssetId::get(), &who, T::StorageFees::get()).is_ok(), + T::Currency::can_withdraw(T::StorageFeesAssetId::get(), &who, T::StorageFees::get()) + .into_result() + .is_ok(), Error::::InsufficientBalance ); @@ -484,6 +505,7 @@ pub mod pallet { &who, &T::StorageFeesBeneficiary::get(), T::StorageFees::get(), + false, )?; } @@ -505,13 +527,11 @@ impl Pallet { /// Convert Vec to BoundedVec so it respects the max set limit, otherwise return TooLong error fn try_into_bounded(name: Option>) -> Result>, Error> { - if let Some(name) = name { - TryInto::>::try_into(name) - .map_err(|_| Error::::TooLong) - .map(Some) - } else { - Ok(None) - } + let Some(s) = name else { + return Ok(None); + }; + + s.try_into().map_err(|_| Error::::TooLong).map(Some) } fn do_set_location(asset_id: T::AssetId, location: T::AssetNativeLocation) -> Result<(), DispatchError> { @@ -587,8 +607,11 @@ impl Pallet { ) -> Result { let bounded_name = Self::try_into_bounded(Some(name))?; - //NOTE: this unwrap is safe. - if let Some(asset_id) = AssetIds::::get(bounded_name.as_ref().unwrap()) { + if let Some(asset_id) = AssetIds::::get( + bounded_name + .as_ref() + .ok_or_else(|| -> pallet::Error { InconsistentStateError::BoundedConversionFailed.into() })?, + ) { Ok(asset_id) } else { Self::do_register_asset( @@ -624,8 +647,8 @@ impl Registry, Balance, DispatchError> for Pallet } fn retrieve_asset(name: &Vec) -> Result { - //NOTE: This unwrap is safe. - let bounded_name = Self::try_into_bounded(Some(name.clone()))?.unwrap(); + let bounded_name = Self::try_into_bounded(Some(name.clone()))? + .ok_or_else(|| -> pallet::Error { InconsistentStateError::BoundedConversionFailed.into() })?; if let Some(asset_id) = AssetIds::::get(bounded_name) { Ok(asset_id) } else { diff --git a/pallets/asset-registry/src/tests/register.rs b/pallets/asset-registry/src/tests/register.rs index 766af4159..b5bcfbba2 100644 --- a/pallets/asset-registry/src/tests/register.rs +++ b/pallets/asset-registry/src/tests/register.rs @@ -2,6 +2,7 @@ use super::*; use crate::types::AssetType; use frame_support::error::BadOrigin; +use frame_support::traits::tokens::fungibles::Mutate as MutateFungibles; use mock::{AssetId, Registry}; use polkadot_xcm::v3::{ Junction::{self, Parachain}, @@ -382,8 +383,8 @@ fn register_external_asset_should_work_when_location_is_provided() { let asset_location = AssetLocation(MultiLocation::new(0, X2(Parachain(200), key))); let alice_balance = 10_000 * UNIT; - Tokens::set_balance(RuntimeOrigin::root(), ALICE, NativeAssetId::get(), alice_balance, 0).unwrap(); - assert_eq!(Tokens::free_balance(NativeAssetId::get(), &TREASURY), 0); + Tokens::mint_into(NativeAssetId::get(), &ALICE, alice_balance).unwrap(); + assert_eq!(Tokens::balance(NativeAssetId::get(), &TREASURY), 0); //Act assert_ok!(Registry::register_external( @@ -431,10 +432,10 @@ fn register_external_asset_should_work_when_location_is_provided() { )); assert_eq!( - Tokens::free_balance(NativeAssetId::get(), &ALICE), + Tokens::balance(NativeAssetId::get(), &ALICE), alice_balance - StoreFees::get() ); - assert_eq!(Tokens::free_balance(NativeAssetId::get(), &TREASURY), StoreFees::get()); + assert_eq!(Tokens::balance(NativeAssetId::get(), &TREASURY), StoreFees::get()); }); } @@ -467,8 +468,8 @@ fn register_external_asset_should_not_work_when_location_is_already_used() { )); let alice_balance = 10_000 * UNIT; - Tokens::set_balance(RuntimeOrigin::root(), ALICE, NativeAssetId::get(), alice_balance, 0).unwrap(); - assert_eq!(Tokens::free_balance(NativeAssetId::get(), &TREASURY), 0); + Tokens::mint_into(NativeAssetId::get(), &ALICE, alice_balance).unwrap(); + assert_eq!(Tokens::balance(NativeAssetId::get(), &TREASURY), 0); //Act assert_noop!( diff --git a/pallets/xyk/src/benchmarking.rs b/pallets/xyk/src/benchmarking.rs index 122ddfea3..6acf6cb85 100644 --- a/pallets/xyk/src/benchmarking.rs +++ b/pallets/xyk/src/benchmarking.rs @@ -32,7 +32,7 @@ const SEED: u32 = 1; fn funded_account(name: &'static str, index: u32) -> T::AccountId { let caller: T::AccountId = account(name, index, SEED); - //Necessary for ED for insufficien assets. + //Necessary for ED for insufficient assets. T::Currency::update_balance(0, &caller, 1_000_000_000_000_000).unwrap(); T::Currency::update_balance(1, &caller, 1_000_000_000_000_000).unwrap(); diff --git a/runtime/hydradx/src/assets.rs b/runtime/hydradx/src/assets.rs index 716fcd184..a22d3af97 100644 --- a/runtime/hydradx/src/assets.rs +++ b/runtime/hydradx/src/assets.rs @@ -346,7 +346,7 @@ impl pallet_asset_registry::Config for Runtime { type RuntimeEvent = RuntimeEvent; type RegistryOrigin = EnsureRoot; type UpdateOrigin = SuperMajorityTechCommittee; - type Currency = Currencies; + type Currency = pallet_currencies::fungibles::FungibleCurrencies; type AssetId = AssetId; type AssetNativeLocation = AssetLocation; type StringLimit = RegistryStrLimit; diff --git a/runtime/hydradx/src/benchmarking/route_executor.rs b/runtime/hydradx/src/benchmarking/route_executor.rs index a4e3e1cde..89aeaff02 100644 --- a/runtime/hydradx/src/benchmarking/route_executor.rs +++ b/runtime/hydradx/src/benchmarking/route_executor.rs @@ -18,7 +18,6 @@ use crate::{AccountId, AssetId, Balance, Currencies, Router, Runtime, System, LBP}; -use crate::InsufficientEDinHDX; use frame_benchmarking::account; use frame_support::dispatch::DispatchResult; use frame_support::{assert_ok, ensure}; @@ -129,7 +128,7 @@ runtime_benchmarks! { assert_eq!(>::free_balance( asset_in, &seller, - ), INITIAL_BALANCE - amount_to_sell - InsufficientEDinHDX::get()); + ), INITIAL_BALANCE - amount_to_sell); } } From 5a03cf08945b492be0a05d343e4c42aa2e460b4f Mon Sep 17 00:00:00 2001 From: martinfridrich Date: Wed, 15 Nov 2023 13:53:55 +0100 Subject: [PATCH 2/2] fix PR comments --- .../src/insufficient_assets_ed.rs | 80 ++++++++++--------- runtime/hydradx/src/assets.rs | 6 +- 2 files changed, 45 insertions(+), 41 deletions(-) diff --git a/integration-tests/src/insufficient_assets_ed.rs b/integration-tests/src/insufficient_assets_ed.rs index 51ab7224e..4ef19b1eb 100644 --- a/integration-tests/src/insufficient_assets_ed.rs +++ b/integration-tests/src/insufficient_assets_ed.rs @@ -34,7 +34,7 @@ fn sender_should_pay_ed_in_hdx_when_it_is_not_whitelisted() { let treasury_balance = Currencies::free_balance(HDX, &TreasuryAccount::get()); assert_eq!(Currencies::free_balance(sht1, &ALICE.into()), 0); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); //Act assert_ok!(Tokens::transfer( @@ -57,7 +57,7 @@ fn sender_should_pay_ed_in_hdx_when_it_is_not_whitelisted() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_balance + InsufficientEDinHDX::get() ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), @@ -76,7 +76,7 @@ fn sender_should_pay_ed_in_hdx_when_it_is_not_whitelisted() { } #[test] -fn reciever_should_pay_ed_in_hdx_when_insuficcient_asset_was_depositted() { +fn reciever_should_pay_ed_in_hdx_when_insuficcient_asset_was_deposited() { TestNet::reset(); Hydra::execute_with(|| { let sht1: AssetId = register_external_asset(0_u128); @@ -92,7 +92,7 @@ fn reciever_should_pay_ed_in_hdx_when_insuficcient_asset_was_depositted() { let treasury_balance = Currencies::free_balance(HDX, &TreasuryAccount::get()); assert_eq!(Currencies::free_balance(sht1, &ALICE.into()), 0); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); //Act assert_ok!(Tokens::deposit(sht1, &ALICE.into(), 1_000_000 * UNITS)); @@ -109,7 +109,7 @@ fn reciever_should_pay_ed_in_hdx_when_insuficcient_asset_was_depositted() { treasury_balance + InsufficientEDinHDX::get() ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), @@ -145,7 +145,7 @@ fn hdx_ed_should_be_released_when_account_is_killed_and_ed_was_paid_in_hdx() { let alice_balance = Currencies::free_balance(HDX, &ALICE.into()); let treasury_balance = Currencies::free_balance(HDX, &TreasuryAccount::get()); - assert_eq!(treasury_suffyciency_lock(), 1100000000000_u128); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); //Act assert_ok!(Tokens::transfer( @@ -167,7 +167,7 @@ fn hdx_ed_should_be_released_when_account_is_killed_and_ed_was_paid_in_hdx() { treasury_balance - InsufficientEDinHDX::get() ); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), @@ -251,7 +251,7 @@ fn sender_should_pay_ed_only_when_dest_didnt_pay_yet() { .unwrap() .saturating_mul_int(InsufficientEDinHDX::get()); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), @@ -319,7 +319,7 @@ fn dest_should_pay_ed_only_once_when_insufficient_asset_was_depsitted() { .unwrap() .saturating_mul_int(InsufficientEDinHDX::get()); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), @@ -372,7 +372,7 @@ fn hdx_ed_should_be_released_when_account_is_killed_and_ed_was_paid_in_fee_asset let treasury_hdx_balance = Currencies::free_balance(HDX, &TreasuryAccount::get()); let treasury_fee_asset_balance = Currencies::free_balance(fee_asset, &TreasuryAccount::get()); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); //Act assert_ok!(Tokens::transfer( @@ -403,7 +403,7 @@ fn hdx_ed_should_be_released_when_account_is_killed_and_ed_was_paid_in_fee_asset treasury_fee_asset_balance ); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), @@ -479,7 +479,7 @@ fn sender_should_pay_ed_in_fee_asset_when_sending_insufficient_asset() { let treasury_fee_asset_balance = Currencies::free_balance(fee_asset, &TreasuryAccount::get()); assert_eq!(Currencies::free_balance(sht1, &ALICE.into()), 0); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); //Act assert_ok!(Tokens::transfer( @@ -513,7 +513,7 @@ fn sender_should_pay_ed_in_fee_asset_when_sending_insufficient_asset() { treasury_fee_asset_balance + ed_in_fee_asset ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), @@ -533,8 +533,10 @@ fn sender_should_pay_ed_in_fee_asset_when_sending_insufficient_asset() { #[test] fn grandfathered_account_should_receive_hdx_when_account_is_killed() { - //NOTE: thiscase simulates old account that received insufficient asset before sufficiency + //NOTE: this case simulates old account that received insufficient asset before sufficiency //check and didn't pay ED. + //`GRANDFATHERED_UNPAID_ED` bypassed `SufficiencyCheck` by receiving tokens during chain state + //initialization. TestNet::reset(); Hydra::execute_with(|| { @@ -570,7 +572,7 @@ fn grandfathered_account_should_receive_hdx_when_account_is_killed() { ); //NOTE: this is zero because Alice paid ED and it was paid to grandfathered - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 0_u128 @@ -601,7 +603,7 @@ fn ed_should_not_be_collected_when_transfering_or_depositing_sufficient_assets() let alice_sufficient_asset_balance = Currencies::free_balance(DAI, &ALICE.into()); let treasury_hdx_balance = Currencies::free_balance(HDX, &TreasuryAccount::get()); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -626,7 +628,7 @@ fn ed_should_not_be_collected_when_transfering_or_depositing_sufficient_assets() Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -651,7 +653,7 @@ fn ed_should_not_be_collected_when_transfering_or_depositing_sufficient_assets() Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -686,7 +688,7 @@ fn ed_should_not_be_released_when_sufficient_asset_killed_account() { let alice_sufficient_asset_balance = Currencies::free_balance(DAI, &ALICE.into()); let treasury_hdx_balance = Currencies::free_balance(HDX, &TreasuryAccount::get()); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -714,7 +716,7 @@ fn ed_should_not_be_released_when_sufficient_asset_killed_account() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -746,7 +748,7 @@ fn ed_should_be_collected_for_each_insufficient_asset_when_transfered_or_deposit assert_eq!(MultiTransactionPayment::account_currency(&ALICE.into()), HDX); assert_eq!(MultiTransactionPayment::account_currency(&BOB.into()), HDX); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_ok!(Tokens::set_balance( RawOrigin::Root.into(), @@ -811,7 +813,7 @@ fn ed_should_be_collected_for_each_insufficient_asset_when_transfered_or_deposit Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance + InsufficientEDinHDX::get() * 4 ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get() * 4); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get() * 4); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 4_u128 @@ -868,7 +870,7 @@ fn ed_should_be_released_for_each_insufficient_asset_when_account_is_killed() { let alice_hdx_balance = Currencies::free_balance(HDX, &ALICE.into()); let treasury_hdx_balance = Currencies::free_balance(HDX, &TreasuryAccount::get()); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get() * 4); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get() * 4); //Act 1 assert_ok!(Tokens::transfer( @@ -887,7 +889,7 @@ fn ed_should_be_released_for_each_insufficient_asset_when_account_is_killed() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance - InsufficientEDinHDX::get() ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get() * 3); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get() * 3); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 3_u128 @@ -910,7 +912,7 @@ fn ed_should_be_released_for_each_insufficient_asset_when_account_is_killed() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance - InsufficientEDinHDX::get() * 2 ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get() * 2); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get() * 2); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 2_u128 @@ -933,7 +935,7 @@ fn ed_should_be_released_for_each_insufficient_asset_when_account_is_killed() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance - InsufficientEDinHDX::get() * 3 ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -956,7 +958,7 @@ fn ed_should_be_released_for_each_insufficient_asset_when_account_is_killed() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance - InsufficientEDinHDX::get() * 4 ); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 0_u128 @@ -1008,7 +1010,7 @@ fn mix_of_sufficinet_and_insufficient_assets_should_lock_unlock_ed_correctly() { let alice_hdx_balance = Currencies::free_balance(HDX, &ALICE.into()); let treasury_hdx_balance = Currencies::free_balance(HDX, &TreasuryAccount::get()); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get() * 2); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get() * 2); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 2_u128 @@ -1031,7 +1033,7 @@ fn mix_of_sufficinet_and_insufficient_assets_should_lock_unlock_ed_correctly() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance - InsufficientEDinHDX::get() ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -1056,7 +1058,7 @@ fn mix_of_sufficinet_and_insufficient_assets_should_lock_unlock_ed_correctly() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance ); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -1083,7 +1085,7 @@ fn mix_of_sufficinet_and_insufficient_assets_should_lock_unlock_ed_correctly() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance - InsufficientEDinHDX::get() ); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 0_u128 @@ -1107,7 +1109,7 @@ fn mix_of_sufficinet_and_insufficient_assets_should_lock_unlock_ed_correctly() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance ); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 0_u128 @@ -1131,7 +1133,7 @@ fn mix_of_sufficinet_and_insufficient_assets_should_lock_unlock_ed_correctly() { Currencies::free_balance(HDX, &TreasuryAccount::get()), treasury_hdx_balance ); - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 0_u128 @@ -1180,7 +1182,7 @@ fn sender_should_pay_ed_when_tranferred_or_deposited_to_whitelisted_dest() { bob_fee_asset_balance - InsufficientEDinHDX::get() ); assert_eq!(Currencies::free_balance(sht1, &treasury), 10); - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -1197,7 +1199,7 @@ fn sender_should_pay_ed_when_tranferred_or_deposited_to_whitelisted_dest() { assert_eq!(Currencies::free_balance(sht1, &treasury), 10); assert_eq!(Currencies::free_balance(sht2, &treasury), 20); //NOTE: treasury paid ED in hdx so hdx balance didn't changed but locked was increased. - assert_eq!(treasury_suffyciency_lock(), 2 * InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), 2 * InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 2_u128 @@ -1258,7 +1260,7 @@ fn ed_should_be_released_when_whitelisted_account_was_killed() { let treasury_hdx_balance = Currencies::free_balance(HDX, &treasury); //NOTE: set_balance bypass mutation hooks so none was paid. - assert_eq!(treasury_suffyciency_lock(), InsufficientEDinHDX::get()); + assert_eq!(treasury_sufficiency_lock(), InsufficientEDinHDX::get()); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 1_u128 @@ -1278,7 +1280,7 @@ fn ed_should_be_released_when_whitelisted_account_was_killed() { assert_eq!(Currencies::free_balance(sht1, &BOB.into()), 2_000_000 * UNITS); //NOTE: bob already holds sht1 so it means additional ed is not necessary. - assert_eq!(treasury_suffyciency_lock(), 0); + assert_eq!(treasury_sufficiency_lock(), 0); assert_eq!( pallet_asset_registry::pallet::ExistentialDepositCounter::::get(), 0_u128 @@ -1347,7 +1349,7 @@ fn register_external_asset(general_index: u128) -> AssetId { next_asset_id } -fn treasury_suffyciency_lock() -> Balance { +fn treasury_sufficiency_lock() -> Balance { pallet_balances::Locks::::get(TreasuryAccount::get()) .iter() .find(|x| x.id == SUFFICIENCY_LOCK) @@ -1359,7 +1361,7 @@ fn treasury_suffyciency_lock() -> Balance { /// /// Parameters: /// - `event` -/// - `times` - number of times event should occure. +/// - `times` - number of times event should occur. #[macro_export] macro_rules! assert_event_times { ( $x:expr, $y: expr ) => {{ diff --git a/runtime/hydradx/src/assets.rs b/runtime/hydradx/src/assets.rs index a22d3af97..1394f5b5b 100644 --- a/runtime/hydradx/src/assets.rs +++ b/runtime/hydradx/src/assets.rs @@ -18,6 +18,7 @@ use super::*; use crate::system::NativeAssetId; +use frame_support::traits::Defensive; use hydradx_adapters::{ inspect::MultiInspectAdapter, EmaOraclePriceAdapter, FreezableNFT, MultiCurrencyLockedBalance, OmnipoolHookAdapter, OracleAssetVolumeProvider, PriceAdjustmentAdapter, StableswapHooksAdapter, VestingInfo, @@ -105,7 +106,7 @@ parameter_types! { pub struct SufficiencyCheck; impl SufficiencyCheck { - /// This function is used by `orml-toknes` `MutationHooks` before a transaction is executed. + /// This function is used by `orml-toknes::MutationHooks` before a transaction is executed. /// It is called from `PreDeposit` and `PreTransfer`. /// If transferred asset is not sufficient asset, it calculates ED amount in user's fee asset /// and transfers it from user to treasury account. @@ -242,7 +243,8 @@ impl Happened<(AccountId, AssetId)> for OnKilledTokenAccount { NativeAssetId::get(), &TreasuryAccount::get(), to_lock, - ); + ) + .defensive(); } let _ = >::transfer(