asset-rate pallet: box asset kind parameter (#1545)

The `AssetKind` type parameter of a dispatchable, defined by the user,
might be large — like `xcm::MultiLocation`. To prevent inflating the
size of the `Call` type, we `Box` it.

This changes required for
https://github.com/paritytech/polkadot-sdk/pull/1333

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

substrate/frame/asset-rate/src/benchmarking.rs

@@ -54,7 +54,7 @@ mod benchmarks {
 	fn create() -> Result<(), BenchmarkError> {
 		let asset_kind: T::AssetKind = T::BenchmarkHelper::create_asset_kind(SEED);
 		#[extrinsic_call]
-		_(RawOrigin::Root, asset_kind.clone(), default_conversion_rate());
+		_(RawOrigin::Root, Box::new(asset_kind.clone()), default_conversion_rate());
 
 		assert_eq!(
 			pallet_asset_rate::ConversionRateToNative::<T>::get(asset_kind),
@@ -68,12 +68,12 @@ mod benchmarks {
 		let asset_kind: T::AssetKind = T::BenchmarkHelper::create_asset_kind(SEED);
 		assert_ok!(AssetRate::<T>::create(
 			RawOrigin::Root.into(),
-			asset_kind.clone(),
+			Box::new(asset_kind.clone()),
 			default_conversion_rate()
 		));
 
 		#[extrinsic_call]
-		_(RawOrigin::Root, asset_kind.clone(), FixedU128::from_u32(2));
+		_(RawOrigin::Root, Box::new(asset_kind.clone()), FixedU128::from_u32(2));
 
 		assert_eq!(
 			pallet_asset_rate::ConversionRateToNative::<T>::get(asset_kind),
@@ -87,12 +87,12 @@ mod benchmarks {
 		let asset_kind: T::AssetKind = T::BenchmarkHelper::create_asset_kind(SEED);
 		assert_ok!(AssetRate::<T>::create(
 			RawOrigin::Root.into(),
-			asset_kind.clone(),
+			Box::new(asset_kind.clone()),
 			default_conversion_rate()
 		));
 
 		#[extrinsic_call]
-		_(RawOrigin::Root, asset_kind.clone());
+		_(RawOrigin::Root, Box::new(asset_kind.clone()));
 
 		assert!(pallet_asset_rate::ConversionRateToNative::<T>::get(asset_kind).is_none());
 		Ok(())

substrate/frame/asset-rate/src/lib.rs

@@ -61,6 +61,7 @@
 
 use frame_support::traits::{fungible::Inspect, tokens::ConversionFromAssetBalance};
 use sp_runtime::{traits::Zero, FixedPointNumber, FixedU128};
+use sp_std::boxed::Box;
 
 pub use pallet::*;
 pub use weights::WeightInfo;
@@ -155,18 +156,18 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::create())]
 		pub fn create(
 			origin: OriginFor<T>,
-			asset_kind: T::AssetKind,
+			asset_kind: Box<T::AssetKind>,
 			rate: FixedU128,
 		) -> DispatchResult {
 			T::CreateOrigin::ensure_origin(origin)?;
 
 			ensure!(
-				!ConversionRateToNative::<T>::contains_key(asset_kind.clone()),
+				!ConversionRateToNative::<T>::contains_key(asset_kind.as_ref()),
 				Error::<T>::AlreadyExists
 			);
-			ConversionRateToNative::<T>::set(asset_kind.clone(), Some(rate));
+			ConversionRateToNative::<T>::set(asset_kind.as_ref(), Some(rate));
 
-			Self::deposit_event(Event::AssetRateCreated { asset_kind, rate });
+			Self::deposit_event(Event::AssetRateCreated { asset_kind: *asset_kind, rate });
 			Ok(())
 		}
 
@@ -178,13 +179,13 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::update())]
 		pub fn update(
 			origin: OriginFor<T>,
-			asset_kind: T::AssetKind,
+			asset_kind: Box<T::AssetKind>,
 			rate: FixedU128,
 		) -> DispatchResult {
 			T::UpdateOrigin::ensure_origin(origin)?;
 
 			let mut old = FixedU128::zero();
-			ConversionRateToNative::<T>::mutate(asset_kind.clone(), |maybe_rate| {
+			ConversionRateToNative::<T>::mutate(asset_kind.as_ref(), |maybe_rate| {
 				if let Some(r) = maybe_rate {
 					old = *r;
 					*r = rate;
@@ -195,7 +196,11 @@ pub mod pallet {
 				}
 			})?;
 
-			Self::deposit_event(Event::AssetRateUpdated { asset_kind, old, new: rate });
+			Self::deposit_event(Event::AssetRateUpdated {
+				asset_kind: *asset_kind,
+				old,
+				new: rate,
+			});
 			Ok(())
 		}
 
@@ -205,16 +210,16 @@ pub mod pallet {
 		/// - O(1)
 		#[pallet::call_index(2)]
 		#[pallet::weight(T::WeightInfo::remove())]
-		pub fn remove(origin: OriginFor<T>, asset_kind: T::AssetKind) -> DispatchResult {
+		pub fn remove(origin: OriginFor<T>, asset_kind: Box<T::AssetKind>) -> DispatchResult {
 			T::RemoveOrigin::ensure_origin(origin)?;
 
 			ensure!(
-				ConversionRateToNative::<T>::contains_key(asset_kind.clone()),
+				ConversionRateToNative::<T>::contains_key(asset_kind.as_ref()),
 				Error::<T>::UnknownAssetKind
 			);
-			ConversionRateToNative::<T>::remove(asset_kind.clone());
+			ConversionRateToNative::<T>::remove(asset_kind.as_ref());
 
-			Self::deposit_event(Event::AssetRateRemoved { asset_kind });
+			Self::deposit_event(Event::AssetRateRemoved { asset_kind: *asset_kind });
 			Ok(())
 		}
 	}

substrate/frame/asset-rate/src/tests.rs

@@ -29,7 +29,11 @@ const ASSET_ID: u32 = 42;
 fn create_works() {
 	new_test_ext().execute_with(|| {
 		assert!(pallet_asset_rate::ConversionRateToNative::<Test>::get(ASSET_ID).is_none());
-		assert_ok!(AssetRate::create(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.1)));
+		assert_ok!(AssetRate::create(
+			RuntimeOrigin::root(),
+			Box::new(ASSET_ID),
+			FixedU128::from_float(0.1)
+		));
 
 		assert_eq!(
 			pallet_asset_rate::ConversionRateToNative::<Test>::get(ASSET_ID),
@@ -42,10 +46,18 @@ fn create_works() {
 fn create_existing_throws() {
 	new_test_ext().execute_with(|| {
 		assert!(pallet_asset_rate::ConversionRateToNative::<Test>::get(ASSET_ID).is_none());
-		assert_ok!(AssetRate::create(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.1)));
+		assert_ok!(AssetRate::create(
+			RuntimeOrigin::root(),
+			Box::new(ASSET_ID),
+			FixedU128::from_float(0.1)
+		));
 
 		assert_noop!(
-			AssetRate::create(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.1)),
+			AssetRate::create(
+				RuntimeOrigin::root(),
+				Box::new(ASSET_ID),
+				FixedU128::from_float(0.1)
+			),
 			Error::<Test>::AlreadyExists
 		);
 	});
@@ -54,9 +66,13 @@ fn create_existing_throws() {
 #[test]
 fn remove_works() {
 	new_test_ext().execute_with(|| {
-		assert_ok!(AssetRate::create(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.1)));
+		assert_ok!(AssetRate::create(
+			RuntimeOrigin::root(),
+			Box::new(ASSET_ID),
+			FixedU128::from_float(0.1)
+		));
 
-		assert_ok!(AssetRate::remove(RuntimeOrigin::root(), ASSET_ID,));
+		assert_ok!(AssetRate::remove(RuntimeOrigin::root(), Box::new(ASSET_ID),));
 		assert!(pallet_asset_rate::ConversionRateToNative::<Test>::get(ASSET_ID).is_none());
 	});
 }
@@ -65,7 +81,7 @@ fn remove_works() {
 fn remove_unknown_throws() {
 	new_test_ext().execute_with(|| {
 		assert_noop!(
-			AssetRate::remove(RuntimeOrigin::root(), ASSET_ID,),
+			AssetRate::remove(RuntimeOrigin::root(), Box::new(ASSET_ID),),
 			Error::<Test>::UnknownAssetKind
 		);
 	});
@@ -74,8 +90,16 @@ fn remove_unknown_throws() {
 #[test]
 fn update_works() {
 	new_test_ext().execute_with(|| {
-		assert_ok!(AssetRate::create(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.1)));
-		assert_ok!(AssetRate::update(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.5)));
+		assert_ok!(AssetRate::create(
+			RuntimeOrigin::root(),
+			Box::new(ASSET_ID),
+			FixedU128::from_float(0.1)
+		));
+		assert_ok!(AssetRate::update(
+			RuntimeOrigin::root(),
+			Box::new(ASSET_ID),
+			FixedU128::from_float(0.5)
+		));
 
 		assert_eq!(
 			pallet_asset_rate::ConversionRateToNative::<Test>::get(ASSET_ID),
@@ -88,7 +112,11 @@ fn update_works() {
 fn update_unknown_throws() {
 	new_test_ext().execute_with(|| {
 		assert_noop!(
-			AssetRate::update(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.5)),
+			AssetRate::update(
+				RuntimeOrigin::root(),
+				Box::new(ASSET_ID),
+				FixedU128::from_float(0.5)
+			),
 			Error::<Test>::UnknownAssetKind
 		);
 	});
@@ -97,7 +125,11 @@ fn update_unknown_throws() {
 #[test]
 fn convert_works() {
 	new_test_ext().execute_with(|| {
-		assert_ok!(AssetRate::create(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(2.51)));
+		assert_ok!(AssetRate::create(
+			RuntimeOrigin::root(),
+			Box::new(ASSET_ID),
+			FixedU128::from_float(2.51)
+		));
 
 		let conversion = <AssetRate as ConversionFromAssetBalance<
 			BalanceOf<Test>,