diff --git a/pallets/asset-registry/src/benchmarking.rs b/pallets/asset-registry/src/benchmarking.rs index d5df544a..93ad89ad 100644 --- a/pallets/asset-registry/src/benchmarking.rs +++ b/pallets/asset-registry/src/benchmarking.rs @@ -1,11 +1,10 @@ //! Benchmarking setup for pallet-asset-registry - 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}, @@ -15,38 +14,28 @@ use xcm::opaque::latest::{ 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 24ccc330..e2680e06 100644 --- a/pallets/asset-registry/src/lib.rs +++ b/pallets/asset-registry/src/lib.rs @@ -32,21 +32,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,7 +87,7 @@ pub mod pallet { 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!( @@ -90,19 +96,20 @@ 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 }); @@ -118,12 +125,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 }); @@ -140,11 +146,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/pallets/asset-registry/src/weights.rs b/pallets/asset-registry/src/weights.rs index 8074c5b4..4d429a7b 100644 --- a/pallets/asset-registry/src/weights.rs +++ b/pallets/asset-registry/src/weights.rs @@ -1,12 +1,10 @@ - //! Autogenerated weights for `pallet_asset_registry` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2022-10-04, STEPS: `20`, REPEAT: 10, LOW RANGE: `[]`, HIGH RANGE: `[]` -//! HOSTNAME: `bernardo-benchmarking`, CPU: `AMD EPYC 7B13` -//! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 - -// collected on a c2d-highcpu-8 of Google Cloud Platform +//! DATE: 2023-05-18, STEPS: `20`, REPEAT: `10`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `vale`, CPU: `11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz` +//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: // ./target/release/trappist-collator @@ -16,6 +14,8 @@ // dev // --pallet // pallet_asset_registry +// --execution=wasm +// --wasm-execution=compiled // --extrinsic // * // --steps @@ -40,37 +40,67 @@ pub trait WeightInfo { /// Weight functions for `pallet_asset_registry`. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - // Storage: Assets Asset (r:1 w:0) - // Storage: AssetRegistry AssetIdMultiLocation (r:1 w:1) - // Storage: AssetRegistry AssetMultiLocationId (r:0 w:1) + /// Storage: Assets Asset (r:1 w:0) + /// Proof: Assets Asset (max_values: None, max_size: Some(210), added: 2685, mode: MaxEncodedLen) + /// Storage: AssetRegistry AssetIdMultiLocation (r:1 w:1) + /// Proof: AssetRegistry AssetIdMultiLocation (max_values: None, max_size: Some(622), added: 3097, mode: MaxEncodedLen) + /// Storage: AssetRegistry AssetMultiLocationId (r:0 w:1) + /// Proof: AssetRegistry AssetMultiLocationId (max_values: None, max_size: Some(622), added: 3097, mode: MaxEncodedLen) fn register_reserve_asset() -> Weight { - Weight::from_parts(18_710_000, 0) + // Proof Size summary in bytes: + // Measured: `123` + // Estimated: `7762` + // Minimum execution time: 21_998_000 picoseconds. + Weight::from_parts(22_970_000, 0) + .saturating_add(Weight::from_parts(0, 7762)) .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(2)) } - // Storage: AssetRegistry AssetIdMultiLocation (r:1 w:1) - // Storage: AssetRegistry AssetMultiLocationId (r:0 w:1) + /// Storage: AssetRegistry AssetIdMultiLocation (r:1 w:1) + /// Proof: AssetRegistry AssetIdMultiLocation (max_values: None, max_size: Some(622), added: 3097, mode: MaxEncodedLen) + /// Storage: AssetRegistry AssetMultiLocationId (r:0 w:1) + /// Proof: AssetRegistry AssetMultiLocationId (max_values: None, max_size: Some(622), added: 3097, mode: MaxEncodedLen) fn unregister_reserve_asset() -> Weight { - Weight::from_parts(16_570_000, 0) + // Proof Size summary in bytes: + // Measured: `107` + // Estimated: `4087` + // Minimum execution time: 17_862_000 picoseconds. + Weight::from_parts(18_454_000, 0) + .saturating_add(Weight::from_parts(0, 4087)) .saturating_add(T::DbWeight::get().reads(1)) .saturating_add(T::DbWeight::get().writes(2)) } } impl WeightInfo for () { - // Storage: Assets Asset (r:1 w:0) - // Storage: AssetRegistry AssetIdMultiLocation (r:1 w:1) - // Storage: AssetRegistry AssetMultiLocationId (r:0 w:1) + /// Storage: Assets Asset (r:1 w:0) + /// Proof: Assets Asset (max_values: None, max_size: Some(210), added: 2685, mode: MaxEncodedLen) + /// Storage: AssetRegistry AssetIdMultiLocation (r:1 w:1) + /// Proof: AssetRegistry AssetIdMultiLocation (max_values: None, max_size: Some(622), added: 3097, mode: MaxEncodedLen) + /// Storage: AssetRegistry AssetMultiLocationId (r:0 w:1) + /// Proof: AssetRegistry AssetMultiLocationId (max_values: None, max_size: Some(622), added: 3097, mode: MaxEncodedLen) fn register_reserve_asset() -> Weight { - Weight::from_parts(18_710_000, 0) + // Proof Size summary in bytes: + // Measured: `123` + // Estimated: `7762` + // Minimum execution time: 21_998_000 picoseconds. + Weight::from_parts(22_970_000, 0) + .saturating_add(Weight::from_parts(0, 7762)) .saturating_add(RocksDbWeight::get().reads(2)) .saturating_add(RocksDbWeight::get().writes(2)) } - // Storage: AssetRegistry AssetIdMultiLocation (r:1 w:1) - // Storage: AssetRegistry AssetMultiLocationId (r:0 w:1) + /// Storage: AssetRegistry AssetIdMultiLocation (r:1 w:1) + /// Proof: AssetRegistry AssetIdMultiLocation (max_values: None, max_size: Some(622), added: 3097, mode: MaxEncodedLen) + /// Storage: AssetRegistry AssetMultiLocationId (r:0 w:1) + /// Proof: AssetRegistry AssetMultiLocationId (max_values: None, max_size: Some(622), added: 3097, mode: MaxEncodedLen) fn unregister_reserve_asset() -> Weight { - Weight::from_parts(16_570_000, 0) + // Proof Size summary in bytes: + // Measured: `107` + // Estimated: `4087` + // Minimum execution time: 17_862_000 picoseconds. + Weight::from_parts(18_454_000, 0) + .saturating_add(Weight::from_parts(0, 4087)) .saturating_add(RocksDbWeight::get().reads(1)) .saturating_add(RocksDbWeight::get().writes(2)) } -} +} \ No newline at end of file diff --git a/runtime/trappist/src/lib.rs b/runtime/trappist/src/lib.rs index c76bcbc6..eeb7118b 100644 --- a/runtime/trappist/src/lib.rs +++ b/runtime/trappist/src/lib.rs @@ -360,7 +360,7 @@ impl pallet_assets::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = AssetBalance; type AssetId = AssetIdForTrustBackedAssets; - type AssetIdParameter = codec::Compact; + type AssetIdParameter = codec::Compact; type Currency = Balances; type CreateOrigin = AsEnsureOriginWithArg>; type ForceOrigin = AssetsForceOrigin; @@ -561,6 +561,34 @@ 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() -> AssetIdForTrustBackedAssets { + 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! { pub const ChessPalletId: PalletId = PalletId(*b"subchess"); pub const IncentiveShare: u8 = 10; // janitor gets 10% of the prize @@ -579,13 +607,6 @@ impl pallet_chess::Config for Runtime { type IncentiveShare = IncentiveShare; } -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; -} - type TreasuryApproveCancelOrigin = EitherOfDiverse< EnsureRoot, pallet_collective::EnsureProportionAtLeast,