diff --git a/pallets/asset-registry/src/benchmarking.rs b/pallets/asset-registry/src/benchmarking.rs index d5df544a..a5b6d7da 100644 --- a/pallets/asset-registry/src/benchmarking.rs +++ b/pallets/asset-registry/src/benchmarking.rs @@ -5,48 +5,37 @@ use super::*; #[allow(unused)] use crate::Pallet as AssetRegistry; use frame_benchmarking::benchmarks; -use frame_support::{assert_ok, traits::fungibles::Inspect}; +use frame_support::assert_ok; use frame_system::RawOrigin; use xcm::opaque::latest::{ Junction::{GeneralIndex, PalletInstance, Parachain}, Junctions, MultiLocation, }; -pub const LOCAL_ASSET_ID: u32 = 10; - benchmarks! { - where_clause { - where - T::Assets: Inspect<::AccountId, AssetId = u32>, - } - register_reserve_asset { + let asset_id = T::BenchmarkHelper::get_registered_asset(); let asset_multi_location = MultiLocation { parents: 1, interior: Junctions::X3(Parachain(Default::default()), PalletInstance(Default::default()), GeneralIndex(Default::default())) }; - - }: _(RawOrigin::Root, LOCAL_ASSET_ID, asset_multi_location.clone()) + }: _(RawOrigin::Root, asset_id, asset_multi_location.clone()) verify { - let read_asset_multi_location = AssetRegistry::::asset_id_multilocation(LOCAL_ASSET_ID) - .expect("error reading AssetIdMultiLocation"); - assert_eq!(read_asset_multi_location, asset_multi_location); + assert_eq!(AssetIdMultiLocation::::get(asset_id), Some(asset_multi_location)); } unregister_reserve_asset { + let asset_id = T::BenchmarkHelper::get_registered_asset(); let asset_multi_location = MultiLocation { parents: 1, interior: Junctions::X3(Parachain(Default::default()), PalletInstance(Default::default()), GeneralIndex(Default::default())) }; - assert_ok!(AssetRegistry::::register_reserve_asset(RawOrigin::Root.into(), LOCAL_ASSET_ID, asset_multi_location.clone())); - let read_asset_multi_location = AssetRegistry::::asset_id_multilocation(LOCAL_ASSET_ID) - .expect("error reading AssetIdMultiLocation"); - assert_eq!(read_asset_multi_location, asset_multi_location); - - }: _(RawOrigin::Root, LOCAL_ASSET_ID) + assert_ok!(AssetRegistry::::register_reserve_asset(RawOrigin::Root.into(), asset_id, asset_multi_location.clone())); + assert!(AssetIdMultiLocation::::contains_key(asset_id)); + }: _(RawOrigin::Root, asset_id) verify { - assert_eq!(AssetRegistry::::asset_id_multilocation(LOCAL_ASSET_ID), None); + assert_eq!(AssetIdMultiLocation::::get(asset_id), None); } impl_benchmark_test_suite!(AssetRegistry, crate::mock::new_test_ext(), crate::mock::Test); diff --git a/pallets/asset-registry/src/lib.rs b/pallets/asset-registry/src/lib.rs index d5b89ded..19ec746e 100644 --- a/pallets/asset-registry/src/lib.rs +++ b/pallets/asset-registry/src/lib.rs @@ -33,21 +33,27 @@ pub mod pallet { type AssetIdOf = <::Assets as Inspect<::AccountId>>::AssetId; + #[cfg(feature = "runtime-benchmarks")] + pub trait BenchmarkHelper { + fn get_registered_asset() -> AssetId; + } + #[pallet::config] pub trait Config: frame_system::Config { type RuntimeEvent: From> + IsType<::RuntimeEvent>; type ReserveAssetModifierOrigin: EnsureOrigin; type Assets: Inspect; type WeightInfo: WeightInfo; + /// Helper trait for benchmarks. + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper: BenchmarkHelper>; } #[pallet::storage] - #[pallet::getter(fn asset_id_multilocation)] pub type AssetIdMultiLocation = StorageMap<_, Blake2_128Concat, AssetIdOf, MultiLocation>; #[pallet::storage] - #[pallet::getter(fn asset_multilocation_id)] pub type AssetMultiLocationId = StorageMap<_, Blake2_128Concat, MultiLocation, AssetIdOf>; @@ -81,8 +87,7 @@ pub mod pallet { ) -> DispatchResult { T::ReserveAssetModifierOrigin::ensure_origin(origin)?; - // verify asset exists on pallet-assets - ensure!(Self::asset_exists(asset_id), Error::::AssetDoesNotExist); + ensure!(T::Assets::asset_exists(asset_id), Error::::AssetDoesNotExist); // verify asset is not yet registered ensure!( @@ -91,23 +96,23 @@ pub mod pallet { ); // verify MultiLocation is valid - let parents_multi_location_ok = { asset_multi_location.parents == 1 }; - let junctions_multi_location_ok = matches!( - asset_multi_location.interior, - Junctions::X3(Parachain(_), PalletInstance(_), GeneralIndex(_)) - ); - ensure!( - parents_multi_location_ok && junctions_multi_location_ok, + matches!( + asset_multi_location, + MultiLocation { + parents: 1, + interior: Junctions::X3(Parachain(_), PalletInstance(_), GeneralIndex(_)) + } + ), Error::::WrongMultiLocation ); - // register asset + // register asset_id => asset_multi_location AssetIdMultiLocation::::insert(asset_id, &asset_multi_location); + // register asset_multi_location => asset_id AssetMultiLocationId::::insert(&asset_multi_location, asset_id); Self::deposit_event(Event::ReserveAssetRegistered { asset_id, asset_multi_location }); - Ok(()) } @@ -119,12 +124,11 @@ pub mod pallet { ) -> DispatchResult { T::ReserveAssetModifierOrigin::ensure_origin(origin)?; - // verify asset is registered + // remove asset_id => asset_multi_location, while getting the value let asset_multi_location = - AssetIdMultiLocation::::get(asset_id).ok_or(Error::::AssetIsNotRegistered)?; - - // unregister asset - AssetIdMultiLocation::::remove(asset_id); + AssetIdMultiLocation::::mutate_exists(asset_id, Option::take) + .ok_or(Error::::AssetIsNotRegistered)?; + // remove asset_multi_location => asset_id AssetMultiLocationId::::remove(&asset_multi_location); Self::deposit_event(Event::ReserveAssetUnregistered { asset_id, asset_multi_location }); @@ -141,11 +145,4 @@ pub mod pallet { AssetMultiLocationId::::get(asset_type) } } - - impl Pallet { - // check if the asset exists - fn asset_exists(asset_id: AssetIdOf) -> bool { - T::Assets::asset_exists(asset_id) - } - } } diff --git a/pallets/asset-registry/src/mock.rs b/pallets/asset-registry/src/mock.rs index cdb31dea..ff8295bc 100644 --- a/pallets/asset-registry/src/mock.rs +++ b/pallets/asset-registry/src/mock.rs @@ -57,11 +57,22 @@ impl system::Config for Test { type MaxConsumers = ConstU32<16>; } +#[cfg(feature = "runtime-benchmarks")] +pub struct MockAssetRegistryBenchmarkHelper; +#[cfg(feature = "runtime-benchmarks")] +impl pallet_asset_registry::BenchmarkHelper for MockAssetRegistryBenchmarkHelper { + fn get_registered_asset() -> u32 { + LOCAL_ASSET_ID + } +} + impl pallet_asset_registry::Config for Test { type RuntimeEvent = RuntimeEvent; type ReserveAssetModifierOrigin = frame_system::EnsureRoot; type Assets = Assets; type WeightInfo = pallet_asset_registry::weights::SubstrateWeight; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = MockAssetRegistryBenchmarkHelper; } impl pallet_balances::Config for Test { diff --git a/pallets/asset-registry/src/tests.rs b/pallets/asset-registry/src/tests.rs index 3d08c598..2b5271ff 100644 --- a/pallets/asset-registry/src/tests.rs +++ b/pallets/asset-registry/src/tests.rs @@ -1,42 +1,67 @@ -use crate::{mock::*, Error}; use frame_support::{assert_noop, assert_ok}; use xcm::latest::prelude::*; +use crate::{mock::*, AssetIdMultiLocation, AssetMultiLocationId, Error}; + +const STATEMINE_ASSET_MULTI_LOCATION: MultiLocation = MultiLocation { + parents: 1, + interior: X3( + Parachain(StatemineParaIdInfo::get()), + PalletInstance(StatemineAssetsInstanceInfo::get()), + GeneralIndex(StatemineAssetIdInfo::get()), + ), +}; + #[test] fn register_reserve_asset_works() { new_test_ext().execute_with(|| { - let statemine_para_id = StatemineParaIdInfo::get(); - let statemine_assets_pallet = StatemineAssetsInstanceInfo::get(); - let statemine_asset_id = StatemineAssetIdInfo::get(); + assert_ok!(AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + LOCAL_ASSET_ID, + STATEMINE_ASSET_MULTI_LOCATION, + )); - let statemine_asset_multi_location = MultiLocation { - parents: 1, - interior: X3( - Parachain(statemine_para_id), - PalletInstance(statemine_assets_pallet), - GeneralIndex(statemine_asset_id), + assert_eq!( + AssetIdMultiLocation::::get(LOCAL_ASSET_ID), + Some(STATEMINE_ASSET_MULTI_LOCATION) + ); + assert_eq!( + AssetMultiLocationId::::get(STATEMINE_ASSET_MULTI_LOCATION), + Some(LOCAL_ASSET_ID) + ); + }); +} + +#[test] +fn cannot_register_unexisting_asset() { + new_test_ext().execute_with(|| { + let unexisting_asset_id = 9999; + + assert_noop!( + AssetRegistry::register_reserve_asset( + RuntimeOrigin::root(), + unexisting_asset_id, + STATEMINE_ASSET_MULTI_LOCATION, ), - }; + Error::::AssetDoesNotExist + ); + }); +} +#[test] +fn cannot_double_register() { + new_test_ext().execute_with(|| { assert_ok!(AssetRegistry::register_reserve_asset( RuntimeOrigin::root(), LOCAL_ASSET_ID, - statemine_asset_multi_location.clone(), + STATEMINE_ASSET_MULTI_LOCATION, )); - let read_asset_multi_location = AssetRegistry::asset_id_multilocation(LOCAL_ASSET_ID) - .expect("error reading AssetIdMultiLocation"); - assert_eq!(read_asset_multi_location, statemine_asset_multi_location); - - let read_asset_id = AssetRegistry::asset_multilocation_id(&statemine_asset_multi_location) - .expect("error reading AssetMultiLocationId"); - assert_eq!(read_asset_id, LOCAL_ASSET_ID); - assert_noop!( AssetRegistry::register_reserve_asset( RuntimeOrigin::root(), LOCAL_ASSET_ID, - statemine_asset_multi_location, + STATEMINE_ASSET_MULTI_LOCATION, ), Error::::AssetAlreadyRegistered ); @@ -46,30 +71,25 @@ fn register_reserve_asset_works() { #[test] fn unregister_reserve_asset_works() { new_test_ext().execute_with(|| { - let statemine_para_id = StatemineParaIdInfo::get(); - let statemine_assets_pallet = StatemineAssetsInstanceInfo::get(); - let statemine_asset_id = StatemineAssetIdInfo::get(); - - let statemine_asset_multi_location = MultiLocation { - parents: 1, - interior: X3( - Parachain(statemine_para_id), - PalletInstance(statemine_assets_pallet), - GeneralIndex(statemine_asset_id), - ), - }; - assert_ok!(AssetRegistry::register_reserve_asset( RuntimeOrigin::root(), LOCAL_ASSET_ID, - statemine_asset_multi_location.clone(), + STATEMINE_ASSET_MULTI_LOCATION, )); assert_ok!(AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID)); - assert!(AssetRegistry::asset_id_multilocation(LOCAL_ASSET_ID).is_none()); - assert!( - AssetRegistry::asset_multilocation_id(statemine_asset_multi_location.clone()).is_none() + assert!(AssetIdMultiLocation::::get(LOCAL_ASSET_ID).is_none()); + assert!(AssetMultiLocationId::::get(STATEMINE_ASSET_MULTI_LOCATION).is_none()); + }); +} + +#[test] +fn cannot_register_unregistered_asset() { + new_test_ext().execute_with(|| { + assert_noop!( + AssetRegistry::unregister_reserve_asset(RuntimeOrigin::root(), LOCAL_ASSET_ID), + Error::::AssetIsNotRegistered ); }); } diff --git a/runtime/trappist/src/lib.rs b/runtime/trappist/src/lib.rs index 3e2be6ff..fc9c1fd1 100644 --- a/runtime/trappist/src/lib.rs +++ b/runtime/trappist/src/lib.rs @@ -361,7 +361,7 @@ impl pallet_assets::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = AssetBalance; type AssetId = AssetId; - type AssetIdParameter = codec::Compact; + type AssetIdParameter = codec::Compact; type Currency = Balances; type CreateOrigin = AsEnsureOriginWithArg>; type ForceOrigin = AssetsForceOrigin; @@ -559,11 +559,30 @@ impl pallet_dex::Config for Runtime { type MinDeposit = ConstU128<{ UNITS }>; } +#[cfg(feature = "runtime-benchmarks")] +pub struct AssetRegistryBenchmarkHelper; +#[cfg(feature = "runtime-benchmarks")] +impl pallet_asset_registry::BenchmarkHelper for AssetRegistryBenchmarkHelper { + fn get_registered_asset() -> AssetId { + use sp_runtime::traits::StaticLookup; + + let root = frame_system::RawOrigin::Root.into(); + let asset_id = 1; + let caller = frame_benchmarking::whitelisted_caller(); + let caller_lookup = ::Lookup::unlookup(caller); + Assets::force_create(root, asset_id.into(), caller_lookup, true, 1) + .expect("Should have been able to force create asset"); + asset_id + } +} + impl pallet_asset_registry::Config for Runtime { type RuntimeEvent = RuntimeEvent; type ReserveAssetModifierOrigin = frame_system::EnsureRoot; type Assets = Assets; type WeightInfo = pallet_asset_registry::weights::SubstrateWeight; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = AssetRegistryBenchmarkHelper; } parameter_types! {