feat(asset-index): move redemption fee to storage

js/pint-types-bundle/index.ts

@@ -116,6 +116,10 @@ export const definitions = {
                     add_proxy: "Weight",
                     remove_proxy: "Weight",
                 },
+                RedemptionFeeRange: {
+                    range: "Vec<BlockNumber>",
+                    fee: "Vec<FeeRate>",
+                },
                 RedemptionState: {
                     _enum: {
                         Initiated: null,

pallets/asset-index/src/benchmarking.rs

@@ -15,7 +15,11 @@ use frame_support::{
 };
 use orml_traits::MultiCurrency;
 use pallet_price_feed::{PriceFeed, PriceFeedBenchmarks};
-use primitives::{traits::NavProvider, AssetAvailability};
+use primitives::{
+	fee::{FeeRate, RedemptionFeeRange},
+	traits::NavProvider,
+	AssetAvailability,
+};
 use xcm::v1::MultiLocation;
 
 use crate::Pallet as AssetIndex;
@@ -60,11 +64,11 @@ benchmarks! {
 
 	complete_withdraw {
 		let asset_id : T::AssetId = T::try_convert(2u8).unwrap();
-		let units = 100_u32.into();
-		let tokens = 500_u32.into();
+		let units = 10_000u32.into();
+		let tokens = 50_000u32.into();
 		let origin = T::AdminOrigin::successful_origin();
 		let origin_account_id = T::AdminOrigin::ensure_origin(origin.clone()).unwrap();
-		let deposit_units = 1000_u32.into();
+		let deposit_units = 1_000_000u32.into();
 
 		// create liquid assets
 		assert_ok!(AssetIndex::<T>::register_asset(
@@ -98,7 +102,7 @@ benchmarks! {
 		// start withdraw
 		assert_ok!(AssetIndex::<T>::withdraw(
 			origin.clone(),
-			42_u32.into(),
+			tokens,
 		));
 		let call = Call::<T>::complete_withdraw();
 	}: { call.dispatch_bypass_filter(origin)? } verify {
@@ -224,11 +228,11 @@ benchmarks! {
 
 	withdraw {
 		let asset_id :T::AssetId =  T::try_convert(2u8).unwrap();
-		let units = 100_u32.into();
-		let tokens = 500_u32.into();
+		let units = 10_000u32.into();
+		let tokens = 50_000u32.into();
 		let origin = T::AdminOrigin::successful_origin();
 		let origin_account_id = T::AdminOrigin::ensure_origin(origin.clone()).unwrap();
-		let deposit_units = 1_000_u32.into();
+		let deposit_units = 1_000_000u32.into();
 
 		// create liquid assets
 		assert_ok!(AssetIndex::<T>::register_asset(
@@ -258,7 +262,7 @@ benchmarks! {
 				+ 1_u32.into(),
 		);
 
-		let call = Call::<T>::withdraw(42_u32.into());
+		let call = Call::<T>::withdraw(tokens);
 	}: { call.dispatch_bypass_filter(origin)? } verify {
 		assert_eq!(pallet::PendingWithdrawals::<T>::get(&origin_account_id).expect("pending withdrawals should be present").len(), 1);
 	}
@@ -304,6 +308,23 @@ benchmarks! {
 	} verify {
 		assert_eq!(pallet::LockupPeriod::<T>::get(), week);
 	}
+
+	set_redemption_fee {
+		let week: T::BlockNumber = (10u32 * 60 * 24 * 7).into();
+		let range = RedemptionFeeRange {
+			range: [week, week * 4u32.into()],
+			fee: [
+				FeeRate { numerator: 1, denominator: 10 },
+				FeeRate { numerator: 1, denominator: 20 },
+				FeeRate { numerator: 1, denominator: 100 }
+			]
+		};
+		let call = Call::<T>::set_redemption_fee(range.clone());
+	}: {
+		call.dispatch_bypass_filter(T::AdminOrigin::successful_origin())?
+	} verify {
+		assert_eq!(pallet::RedemptionFee::<T>::get(), range);
+	}
 }
 
 #[cfg(test)]

pallets/asset-index/src/lib.rs

@@ -51,8 +51,8 @@ pub mod pallet {
 
 	use pallet_price_feed::{AssetPricePair, Price, PriceFeed};
 	use primitives::{
-		fee::{BaseFee, FeeRate},
-		traits::{AssetRecorder, MultiAssetRegistry, NavProvider, RedemptionFee, RemoteAssetManager, SaftRegistry},
+		fee::{BaseFee, FeeRate, RedemptionFeeRange},
+		traits::{AssetRecorder, MultiAssetRegistry, NavProvider, RemoteAssetManager, SaftRegistry},
 		AssetAvailability, AssetProportion, AssetProportions, Ratio,
 	};
 
@@ -112,7 +112,7 @@ pub mod pallet {
 		type MaxDecimals: Get<u8>;
 
 		/// Determines the redemption fee in complete_withdraw
-		type RedemptionFee: RedemptionFee<Self::BlockNumber, Self::Balance>;
+		type RedemptionFee: Get<RedemptionFeeRange<Self::BlockNumber>>;
 
 		/// The native asset id
 		#[pallet::constant]
@@ -155,11 +155,9 @@ pub mod pallet {
 	#[pallet::generate_store(pub (super) trait Store)]
 	pub struct Pallet<T>(_);
 
-	/// Testing storage for RedemptionFee
-	#[cfg(test)]
+	/// stores a range of redemption fee
 	#[pallet::storage]
-	#[pallet::getter(fn last_redemption)]
-	pub type LastRedemption<T: Config> = StorageValue<_, (T::BlockNumber, T::Balance), ValueQuery>;
+	pub type RedemptionFee<T: Config> = StorageValue<_, RedemptionFeeRange<T::BlockNumber>, ValueQuery>;
 
 	/// (AssetId) -> AssetAvailability
 	#[pallet::storage]
@@ -261,6 +259,7 @@ pub mod pallet {
 			use xcm::v1::{Junction, Junctions, MultiLocation};
 
 			LockupPeriod::<T>::set(T::LockupPeriod::get());
+			RedemptionFee::<T>::set(T::RedemptionFee::get());
 
 			for (asset, id) in self.liquid_assets.iter().cloned() {
 				let availability = AssetAvailability::Liquid(MultiLocation {
@@ -311,6 +310,8 @@ pub mod pallet {
 		IndexTokenDepositRangeUpdated(DepositRange<T::Balance>),
 		/// Lockup Period has been updated
 		NewLockupPeriod(T::BlockNumber),
+		/// RedemptionFeeRange has been updated
+		NewRedemptionFeeRange(RedemptionFeeRange<T::BlockNumber>),
 	}
 
 	#[pallet::error]
@@ -327,6 +328,8 @@ pub mod pallet {
 		NoDeposits,
 		/// Invalid metadata given.
 		BadMetadata,
+		/// Thrown when calculate reademption fee failed
+		CalculateRedemptionFeeFailed,
 		/// Thrown if no index could be found for an asset identifier.
 		UnsupportedAsset,
 		/// Thrown if the given amount of PINT to redeem is too low
@@ -349,6 +352,8 @@ pub mod pallet {
 		InvalidDecimals,
 		/// Thrown when the given DepositRange is invalid
 		InvalidDepositRange,
+		/// Thrown when the given RedemptionFeeRange is invalid
+		InvalidRedemptionFeeRange,
 		/// Attempted to set LockupPeriod out of the range of 1 day ~ 28 days
 		InvalidLockupPeriod,
 		/// The deposited amount is below the minimum value required.
@@ -470,9 +475,30 @@ pub mod pallet {
 			Ok(())
 		}
 
-		/// Set lockup period
+		/// Updates the range for redemption fee
 		///
-		/// only accept 1 ~ 28 days
+		/// Only callable by the admin origin
+		///
+		/// Parameters:
+		/// - `new_range`: The new valid range for redemption fee.
+		#[pallet::weight(T::WeightInfo::set_redemption_fee())]
+		pub fn set_redemption_fee(
+			origin: OriginFor<T>,
+			new_range: RedemptionFeeRange<T::BlockNumber>,
+		) -> DispatchResult {
+			T::AdminOrigin::ensure_origin(origin)?;
+			ensure!(new_range.range[0] < new_range.range[1], Error::<T>::InvalidRedemptionFeeRange);
+			RedemptionFee::<T>::set(new_range.clone());
+			Self::deposit_event(Event::<T>::NewRedemptionFeeRange(new_range));
+			Ok(())
+		}
+
+		/// Updates the lockup period
+		///
+		/// Only callable by the admin origin
+		///
+		/// Parameters:
+		/// - `lockup_period`: how long will the depositing assets will be locked
 		#[pallet::weight(T::WeightInfo::set_lockup_period())]
 		pub fn set_lockup_period(origin: OriginFor<T>, lockup_period: T::BlockNumber) -> DispatchResult {
 			T::AdminOrigin::ensure_origin(origin)?;
@@ -847,6 +873,8 @@ pub mod pallet {
 				let mut total_fee: T::Balance = T::Balance::zero();
 				let current_block = frame_system::Pallet::<T>::block_number();
 
+				let redemption_fee_range = RedemptionFee::<T>::get();
+				let mut calculate_redemption_fee_failed = false;
 				let mut rem: Option<(T::Balance, T::BlockNumber)> = None;
 				deposits.retain(|(index_tokens, block_number)| {
 					// how long this deposit spent in the index.
@@ -856,19 +884,32 @@ pub mod pallet {
 						true
 					} else if amount >= *index_tokens {
 						amount = amount.saturating_sub(*index_tokens);
-						total_fee =
-							total_fee.saturating_add(T::RedemptionFee::redemption_fee(time_spent, *index_tokens));
+						if let Some(fee) = redemption_fee_range.redemption_fee(time_spent, *index_tokens) {
+							total_fee = total_fee.saturating_add(fee);
+						} else {
+							calculate_redemption_fee_failed = true;
+						}
+
 						false
 					} else {
 						// the remaining amount is less than the oldest deposit, so we are simply updating the value of
 						// the now oldest deposit
 						rem = Some((index_tokens.saturating_sub(amount), *block_number));
-						total_fee = total_fee.saturating_add(T::RedemptionFee::redemption_fee(time_spent, amount));
+						if let Some(fee) = redemption_fee_range.redemption_fee(time_spent, amount) {
+							total_fee = total_fee.saturating_add(fee);
+						} else {
+							calculate_redemption_fee_failed = true;
+						}
+
 						amount = T::Balance::zero();
 						true
 					}
 				});
 
+				if calculate_redemption_fee_failed {
+					return Err(Error::<T>::CalculateRedemptionFeeFailed.into());
+				}
+
 				if !deposits.is_empty() {
 					if let Some(rem) = rem {
 						// update the oldest value
@@ -1441,6 +1482,7 @@ pub mod pallet {
 		fn set_metadata() -> Weight;
 		fn set_deposit_range() -> Weight;
 		fn set_lockup_period() -> Weight;
+		fn set_redemption_fee() -> Weight;
 	}
 
 	/// For backwards compatibility and tests
@@ -1484,5 +1526,9 @@ pub mod pallet {
 		fn set_lockup_period() -> Weight {
 			Default::default()
 		}
+
+		fn set_redemption_fee() -> Weight {
+			Default::default()
+		}
 	}
 }

pallets/asset-index/src/mock.rs

@@ -24,7 +24,10 @@ use frame_support::{
 use frame_system as system;
 use orml_traits::parameter_type_with_key;
 use pallet_price_feed::PriceFeed;
-use primitives::{fee::FeeRate, AssetPricePair, Price};
+use primitives::{
+	fee::{FeeRate, RedemptionFeeRange},
+	AssetPricePair, Price,
+};
 use sp_core::H256;
 use std::collections::HashMap;
 
@@ -153,20 +156,19 @@ parameter_types! {
 	pub MaxDecimals: u8 = 12;
 	pub MaxActiveDeposits: u32 = 50;
 	pub const PINTAssetId: AssetId = PINT_ASSET_ID;
+	pub const RedemptionFee: RedemptionFeeRange<<Test as system::Config>::BlockNumber> = RedemptionFeeRange {
+		range: [14, 30],
+		fee: [
+			FeeRate { numerator: 1, denominator: 10 },
+			FeeRate { numerator: 1, denominator: 20 },
+			FeeRate { numerator: 1, denominator: 100 }
+		],
+	};
 
 	// No fees for now
 	pub const BaseWithdrawalFee: FeeRate = FeeRate{ numerator: 0, denominator: 1_000,};
 }
 
-pub struct RedemptionFee;
-
-impl primitives::traits::RedemptionFee<BlockNumber, Balance> for RedemptionFee {
-	fn redemption_fee(duration: BlockNumber, amount: Balance) -> Balance {
-		crate::LastRedemption::<Test>::set((duration, amount));
-		Default::default()
-	}
-}
-
 /// Range of lockup period
 pub struct LockupPeriodRange<T>(PhantomData<T>);
 
@@ -294,6 +296,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
 	let mut ext = ExtBuilder::default().build();
 	ext.execute_with(|| {
 		crate::LockupPeriod::<Test>::set(LockupPeriod::get());
+		crate::RedemptionFee::<Test>::set(RedemptionFee::get());
 		System::set_block_number(1)
 	});
 
@@ -320,7 +323,6 @@ pub fn new_test_ext_with_balance(balances: Vec<(AccountId, AssetId, Balance)>) -
 	ext
 }
 
-#[cfg(feature = "runtime-benchmarks")]
 pub fn new_test_ext_from_genesis() -> sp_io::TestExternalities {
 	let mut ext = ExtBuilder::default().build();
 

pallets/asset-index/src/tests.rs

@@ -525,16 +525,21 @@ fn deposit_fails_on_exceeding_limit() {
 
 #[test]
 fn redemption_fee_works_on_completing_withdraw() {
-	let deposit = 1_000;
-	let initial_units = 1_000;
+	let deposit = 1_000_000;
+	let initial_units = 1_000_000;
 
 	new_test_ext().execute_with(|| {
 		assert_ok!(AssetIndex::register_asset(
 			Origin::signed(ACCOUNT_ID),
 			ASSET_A_ID,
 			AssetAvailability::Liquid(MultiLocation::default())
 		));
-		assert_ok!(AssetIndex::add_asset(Origin::signed(ACCOUNT_ID), ASSET_A_ID, 100, initial_units));
+		assert_ok!(AssetIndex::add_asset(
+			Origin::signed(ACCOUNT_ID),
+			ASSET_A_ID,
+			initial_units / ASSET_A_PRICE_MULTIPLIER,
+			initial_units
+		));
 		assert_ok!(Currency::deposit(ASSET_A_ID, &ASHLEY, deposit));
 		for _ in 0..50 {
 			assert_ok!(AssetIndex::deposit(Origin::signed(ASHLEY), ASSET_A_ID, initial_units / 50));
@@ -544,19 +549,18 @@ fn redemption_fee_works_on_completing_withdraw() {
 
 		// advance the block number so that the lock expires
 		let total = AssetIndex::index_token_balance(&ASHLEY);
-		let current_block = frame_system::Pallet::<Test>::block_number();
 		let new_block_number = LockupPeriod::get() + 1;
 		frame_system::Pallet::<Test>::set_block_number(new_block_number);
 		assert_ok!(AssetIndex::withdraw(Origin::signed(ASHLEY), total * 99 / 100));
 		assert_ok!(AssetIndex::complete_withdraw(Origin::signed(ASHLEY)));
-		assert_eq!(Currency::total_balance(ASSET_A_ID, &ASHLEY), deposit * 99 / 100);
 
-		// ensure the redemption fee hook works
-		let index_token_per_deposit = AssetIndex::index_token_equivalent(ASSET_A_ID, deposit).unwrap() / 50;
-		assert_eq!(
-			crate::LastRedemption::<Test>::get(),
-			(new_block_number - current_block, index_token_per_deposit / 2)
-		);
+		// times * deposit_amount * Rate.0 (1 / 10)
+		//
+		// ->
+		//
+		// (50 * (initial_units / 50) - initial_units / 100) * 1 / 10
+		let fee = initial_units * 99 / 1000;
+		assert_eq!(Currency::total_balance(ASSET_A_ID, &ASHLEY), deposit * 99 / 100 - fee);
 
 		// all deposits has been cleaned
 		assert_eq!(crate::Deposits::<Test>::get(&ASHLEY)[0], (total / 100, 1_u64));