From 841a33ede4c991521d5c3ebf852170e978660742 Mon Sep 17 00:00:00 2001
From: Muharem Ismailov <ismailov.m.h@gmail.com>
Date: Fri, 15 Sep 2023 11:52:05 +0200
Subject: [PATCH] asset-rate pallet: box asset kind parameter (#1545)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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>
---
.../frame/asset-rate/src/benchmarking.rs | 10 ++--
substrate/frame/asset-rate/src/lib.rs | 27 ++++++----
substrate/frame/asset-rate/src/tests.rs | 52 +++++++++++++++----
3 files changed, 63 insertions(+), 26 deletions(-)
diff --git a/substrate/frame/asset-rate/src/benchmarking.rs b/substrate/frame/asset-rate/src/benchmarking.rs
index 12edcf5aee0..21d53a89e39 100644
--- a/substrate/frame/asset-rate/src/benchmarking.rs
+++ b/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(())
diff --git a/substrate/frame/asset-rate/src/lib.rs b/substrate/frame/asset-rate/src/lib.rs
index 8b55f3d1d40..c3dc551f876 100644
--- a/substrate/frame/asset-rate/src/lib.rs
+++ b/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(())
}
}
diff --git a/substrate/frame/asset-rate/src/tests.rs b/substrate/frame/asset-rate/src/tests.rs
index 8990ba9fc28..45226547609 100644
--- a/substrate/frame/asset-rate/src/tests.rs
+++ b/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>,
--
GitLab