Skip to content

Commit

Permalink
asset-rate pallet: box asset kind parameter (#1545)
Browse files Browse the repository at this point in the history 
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
#1333

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
@muharem @ggwpez
muharem and ggwpez authored Sep 15, 2023
1 parent b300efc commit 841a33e
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 26 deletions.
10 changes: 5 additions & 5 deletions substrate/frame/asset-rate/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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(())
Expand Down
27 changes: 16 additions & 11 deletions substrate/frame/asset-rate/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
}

Expand All @@ -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;
Expand All @@ -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(())
}

Expand All @@ -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(())
}
}
Expand Down
52 changes: 42 additions & 10 deletions substrate/frame/asset-rate/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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
);
});
Expand All @@ -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());
});
}
Expand All @@ -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
);
});
Expand All @@ -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),
Expand All @@ -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
);
});
Expand All @@ -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>,
Expand Down

0 comments on commit 841a33e

Please sign in to comment.