Pallets: Asset Rate - Rename AssetId to AssetKind, Introduce AssetKindFactory Trait (#14514)

* asset-rate rename asset_id to asset_kind

* asset-rate pallet benchmarks asset kind factory

* rename to SEED

* fixes

* Update frame/asset-rate/src/benchmarking.rs

Co-authored-by: joe petrowski <[email protected]>

* sp-core optional dep

* sp-core included for benchmarks

* sp-core to dev dep

* sp-core add dep prefix

---------

Co-authored-by: joe petrowski <[email protected]>

Author: muharem

bin/node/runtime/src/lib.rs

@@ -1145,9 +1145,11 @@ impl pallet_asset_rate::Config for Runtime {
 	type UpdateOrigin = EnsureRoot<AccountId>;
 	type Balance = Balance;
 	type Currency = Balances;
-	type AssetId = u32;
+	type AssetKind = u32;
 	type RuntimeEvent = RuntimeEvent;
 	type WeightInfo = pallet_asset_rate::weights::SubstrateWeight<Runtime>;
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkHelper = ();
 }
 
 parameter_types! {

frame/asset-rate/Cargo.toml

@@ -22,11 +22,12 @@ frame-support = { version = "4.0.0-dev", default-features = false, path = "../su
 frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" }
 sp-runtime = { version = "24.0.0", default-features = false, path = "../../primitives/runtime" }
 sp-std = { version = "8.0.0", default-features = false, path = "../../primitives/std" }
+sp-core = { version = "21.0.0", default-features = false, optional = true, path = "../../primitives/core" }
 
 [dev-dependencies]
 pallet-balances = { version = "4.0.0-dev", path = "../balances" }
-sp-core = { version = "21.0.0", path = "../../primitives/core" }
 sp-io = { version = "23.0.0", path = "../../primitives/io" }
+sp-core = { version = "21.0.0", default-features = false, path = "../../primitives/core" }
 
 [features]
 default = ["std"]
@@ -38,12 +39,14 @@ std = [
 	"scale-info/std",
 	"sp-runtime/std",
 	"sp-std/std",
+	"sp-core?/std",
 ]
 runtime-benchmarks = [
 	"frame-benchmarking/runtime-benchmarks",
 	"frame-support/runtime-benchmarks",
 	"frame-system/runtime-benchmarks",
 	"sp-runtime/runtime-benchmarks",
+	"dep:sp-core",
 ]
 try-runtime = [
 	"frame-support/try-runtime",

frame/asset-rate/src/benchmarking.rs

@@ -20,65 +20,80 @@
 use super::*;
 use crate::{pallet as pallet_asset_rate, Pallet as AssetRate};
 
+use codec::Encode;
 use frame_benchmarking::v2::*;
 use frame_support::assert_ok;
 use frame_system::RawOrigin;
+use sp_core::crypto::FromEntropy;
 
-const ASSET_ID: u32 = 1;
+/// Trait describing the factory function for the `AssetKind` parameter.
+pub trait AssetKindFactory<AssetKind> {
+	fn create_asset_kind(seed: u32) -> AssetKind;
+}
+impl<AssetKind> AssetKindFactory<AssetKind> for ()
+where
+	AssetKind: FromEntropy,
+{
+	fn create_asset_kind(seed: u32) -> AssetKind {
+		AssetKind::from_entropy(&mut seed.encode().as_slice()).unwrap()
+	}
+}
+
+const SEED: u32 = 1;
 
 fn default_conversion_rate() -> FixedU128 {
 	FixedU128::from_u32(1u32)
 }
 
-#[benchmarks(where <T as Config>::AssetId: From<u32>)]
+#[benchmarks]
 mod benchmarks {
 	use super::*;
 
 	#[benchmark]
 	fn create() -> Result<(), BenchmarkError> {
-		let asset_id: T::AssetId = ASSET_ID.into();
+		let asset_kind: T::AssetKind = T::BenchmarkHelper::create_asset_kind(SEED);
 		#[extrinsic_call]
-		_(RawOrigin::Root, asset_id.clone(), default_conversion_rate());
+		_(RawOrigin::Root, asset_kind.clone(), default_conversion_rate());
 
 		assert_eq!(
-			pallet_asset_rate::ConversionRateToNative::<T>::get(asset_id),
+			pallet_asset_rate::ConversionRateToNative::<T>::get(asset_kind),
 			Some(default_conversion_rate())
 		);
 		Ok(())
 	}
 
 	#[benchmark]
 	fn update() -> Result<(), BenchmarkError> {
-		let asset_id: T::AssetId = ASSET_ID.into();
+		let asset_kind: T::AssetKind = T::BenchmarkHelper::create_asset_kind(SEED);
 		assert_ok!(AssetRate::<T>::create(
 			RawOrigin::Root.into(),
-			asset_id.clone(),
+			asset_kind.clone(),
 			default_conversion_rate()
 		));
 
 		#[extrinsic_call]
-		_(RawOrigin::Root, asset_id.clone(), FixedU128::from_u32(2));
+		_(RawOrigin::Root, asset_kind.clone(), FixedU128::from_u32(2));
 
 		assert_eq!(
-			pallet_asset_rate::ConversionRateToNative::<T>::get(asset_id),
+			pallet_asset_rate::ConversionRateToNative::<T>::get(asset_kind),
 			Some(FixedU128::from_u32(2))
 		);
 		Ok(())
 	}
 
 	#[benchmark]
 	fn remove() -> Result<(), BenchmarkError> {
-		let asset_id: T::AssetId = ASSET_ID.into();
+		let asset_kind: T::AssetKind = T::BenchmarkHelper::create_asset_kind(SEED);
 		assert_ok!(AssetRate::<T>::create(
 			RawOrigin::Root.into(),
-			ASSET_ID.into(),
+			asset_kind.clone(),
 			default_conversion_rate()
 		));
 
 		#[extrinsic_call]
-		_(RawOrigin::Root, asset_id.clone());
+		_(RawOrigin::Root, asset_kind.clone());
 
-		assert!(pallet_asset_rate::ConversionRateToNative::<T>::get(asset_id).is_none());
+		assert!(pallet_asset_rate::ConversionRateToNative::<T>::get(asset_kind).is_none());
 		Ok(())
 	}
 

frame/asset-rate/src/lib.rs

@@ -69,17 +69,19 @@ pub use pallet::*;
 pub use weights::WeightInfo;
 
 #[cfg(feature = "runtime-benchmarks")]
-pub mod benchmarking;
+mod benchmarking;
 #[cfg(test)]
 mod mock;
 #[cfg(test)]
 mod tests;
 pub mod weights;
+#[cfg(feature = "runtime-benchmarks")]
+pub use benchmarking::AssetKindFactory;
 
 // Type alias for `frame_system`'s account id.
 type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
-// This pallet's asset id and balance type.
-type AssetIdOf<T> = <T as Config>::AssetId;
+// This pallet's asset kind and balance type.
+type AssetKindOf<T> = <T as Config>::AssetKind;
 // Generic fungible balance type.
 type BalanceOf<T> = <<T as Config>::Currency as Inspect<AccountIdOf<T>>>::Balance;
 
@@ -115,32 +117,36 @@ pub mod pallet {
 		/// The currency mechanism for this pallet.
 		type Currency: Inspect<Self::AccountId, Balance = Self::Balance>;
 
-		/// The identifier for the class of asset.
-		type AssetId: frame_support::traits::tokens::AssetId;
+		/// The type for asset kinds for which the conversion rate to native balance is set.
+		type AssetKind: Parameter + MaxEncodedLen;
+
+		/// Helper type for benchmarks.
+		#[cfg(feature = "runtime-benchmarks")]
+		type BenchmarkHelper: crate::AssetKindFactory<Self::AssetKind>;
 	}
 
 	/// Maps an asset to its fixed point representation in the native balance.
 	///
-	/// E.g. `native_amount = asset_amount * ConversionRateToNative::<T>::get(asset_id)`
+	/// E.g. `native_amount = asset_amount * ConversionRateToNative::<T>::get(asset_kind)`
 	#[pallet::storage]
 	pub type ConversionRateToNative<T: Config> =
-		StorageMap<_, Blake2_128Concat, T::AssetId, FixedU128, OptionQuery>;
+		StorageMap<_, Blake2_128Concat, T::AssetKind, FixedU128, OptionQuery>;
 
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
-		// Some `asset_id` conversion rate was created.
-		AssetRateCreated { asset_id: T::AssetId, rate: FixedU128 },
-		// Some `asset_id` conversion rate was removed.
-		AssetRateRemoved { asset_id: T::AssetId },
-		// Some existing `asset_id` conversion rate was updated from `old` to `new`.
-		AssetRateUpdated { asset_id: T::AssetId, old: FixedU128, new: FixedU128 },
+		// Some `asset_kind` conversion rate was created.
+		AssetRateCreated { asset_kind: T::AssetKind, rate: FixedU128 },
+		// Some `asset_kind` conversion rate was removed.
+		AssetRateRemoved { asset_kind: T::AssetKind },
+		// Some existing `asset_kind` conversion rate was updated from `old` to `new`.
+		AssetRateUpdated { asset_kind: T::AssetKind, old: FixedU128, new: FixedU128 },
 	}
 
 	#[pallet::error]
 	pub enum Error<T> {
 		/// The given asset ID is unknown.
-		UnknownAssetId,
+		UnknownAssetKind,
 		/// The given asset ID already has an assigned conversion rate and cannot be re-created.
 		AlreadyExists,
 	}
@@ -155,18 +161,18 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::create())]
 		pub fn create(
 			origin: OriginFor<T>,
-			asset_id: T::AssetId,
+			asset_kind: T::AssetKind,
 			rate: FixedU128,
 		) -> DispatchResult {
 			T::CreateOrigin::ensure_origin(origin)?;
 
 			ensure!(
-				!ConversionRateToNative::<T>::contains_key(asset_id.clone()),
+				!ConversionRateToNative::<T>::contains_key(asset_kind.clone()),
 				Error::<T>::AlreadyExists
 			);
-			ConversionRateToNative::<T>::set(asset_id.clone(), Some(rate));
+			ConversionRateToNative::<T>::set(asset_kind.clone(), Some(rate));
 
-			Self::deposit_event(Event::AssetRateCreated { asset_id, rate });
+			Self::deposit_event(Event::AssetRateCreated { asset_kind, rate });
 			Ok(())
 		}
 
@@ -178,24 +184,24 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::update())]
 		pub fn update(
 			origin: OriginFor<T>,
-			asset_id: T::AssetId,
+			asset_kind: T::AssetKind,
 			rate: FixedU128,
 		) -> DispatchResult {
 			T::UpdateOrigin::ensure_origin(origin)?;
 
 			let mut old = FixedU128::zero();
-			ConversionRateToNative::<T>::mutate(asset_id.clone(), |maybe_rate| {
+			ConversionRateToNative::<T>::mutate(asset_kind.clone(), |maybe_rate| {
 				if let Some(r) = maybe_rate {
 					old = *r;
 					*r = rate;
 
 					Ok(())
 				} else {
-					Err(Error::<T>::UnknownAssetId)
+					Err(Error::<T>::UnknownAssetKind)
 				}
 			})?;
 
-			Self::deposit_event(Event::AssetRateUpdated { asset_id, old, new: rate });
+			Self::deposit_event(Event::AssetRateUpdated { asset_kind, old, new: rate });
 			Ok(())
 		}
 
@@ -205,23 +211,23 @@ pub mod pallet {
 		/// - O(1)
 		#[pallet::call_index(2)]
 		#[pallet::weight(T::WeightInfo::remove())]
-		pub fn remove(origin: OriginFor<T>, asset_id: T::AssetId) -> DispatchResult {
+		pub fn remove(origin: OriginFor<T>, asset_kind: T::AssetKind) -> DispatchResult {
 			T::RemoveOrigin::ensure_origin(origin)?;
 
 			ensure!(
-				ConversionRateToNative::<T>::contains_key(asset_id.clone()),
-				Error::<T>::UnknownAssetId
+				ConversionRateToNative::<T>::contains_key(asset_kind.clone()),
+				Error::<T>::UnknownAssetKind
 			);
-			ConversionRateToNative::<T>::remove(asset_id.clone());
+			ConversionRateToNative::<T>::remove(asset_kind.clone());
 
-			Self::deposit_event(Event::AssetRateRemoved { asset_id });
+			Self::deposit_event(Event::AssetRateRemoved { asset_kind });
 			Ok(())
 		}
 	}
 }
 
 /// Exposes conversion of an arbitrary balance of an asset to native balance.
-impl<T> ConversionFromAssetBalance<BalanceOf<T>, AssetIdOf<T>, BalanceOf<T>> for Pallet<T>
+impl<T> ConversionFromAssetBalance<BalanceOf<T>, AssetKindOf<T>, BalanceOf<T>> for Pallet<T>
 where
 	T: Config,
 	BalanceOf<T>: FixedPointOperand + Zero,
@@ -230,10 +236,10 @@ where
 
 	fn from_asset_balance(
 		balance: BalanceOf<T>,
-		asset_id: AssetIdOf<T>,
+		asset_kind: AssetKindOf<T>,
 	) -> Result<BalanceOf<T>, pallet::Error<T>> {
-		let rate = pallet::ConversionRateToNative::<T>::get(asset_id)
-			.ok_or(pallet::Error::<T>::UnknownAssetId.into())?;
+		let rate = pallet::ConversionRateToNative::<T>::get(asset_kind)
+			.ok_or(pallet::Error::<T>::UnknownAssetKind.into())?;
 		Ok(rate.saturating_mul_int(balance))
 	}
 }

frame/asset-rate/src/mock.rs

@@ -86,7 +86,9 @@ impl pallet_asset_rate::Config for Test {
 	type UpdateOrigin = frame_system::EnsureRoot<u64>;
 	type Balance = u64;
 	type Currency = Balances;
-	type AssetId = u32;
+	type AssetKind = u32;
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkHelper = ();
 }
 
 // Build genesis storage according to the mock runtime.

frame/asset-rate/src/tests.rs

@@ -66,7 +66,7 @@ fn remove_unknown_throws() {
 	new_test_ext().execute_with(|| {
 		assert_noop!(
 			AssetRate::remove(RuntimeOrigin::root(), ASSET_ID,),
-			Error::<Test>::UnknownAssetId
+			Error::<Test>::UnknownAssetKind
 		);
 	});
 }
@@ -89,7 +89,7 @@ fn update_unknown_throws() {
 	new_test_ext().execute_with(|| {
 		assert_noop!(
 			AssetRate::update(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.5)),
-			Error::<Test>::UnknownAssetId
+			Error::<Test>::UnknownAssetKind
 		);
 	});
 }
@@ -101,7 +101,7 @@ fn convert_works() {
 
 		let conversion = <AssetRate as ConversionFromAssetBalance<
 			BalanceOf<Test>,
-			<Test as pallet_asset_rate::Config>::AssetId,
+			<Test as pallet_asset_rate::Config>::AssetKind,
 			BalanceOf<Test>,
 		>>::from_asset_balance(10, ASSET_ID);
 		assert_eq!(conversion.expect("Conversion rate exists for asset"), 25);
@@ -113,7 +113,7 @@ fn convert_unknown_throws() {
 	new_test_ext().execute_with(|| {
 		let conversion = <AssetRate as ConversionFromAssetBalance<
 			BalanceOf<Test>,
-			<Test as pallet_asset_rate::Config>::AssetId,
+			<Test as pallet_asset_rate::Config>::AssetKind,
 			BalanceOf<Test>,
 		>>::from_asset_balance(10, ASSET_ID);
 		assert!(conversion.is_err());