From 8e2b7c574c0a86488b7a21cea90ef40809d7623b Mon Sep 17 00:00:00 2001 From: yooml Date: Tue, 10 Sep 2024 13:39:08 +0800 Subject: [PATCH] Update fee share (#1418) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 for review * fix: 🐛 mv storage * fix: 🐛 bench * refactor: 💡 fmt --------- Co-authored-by: Edwin --- pallets/fee-share/Cargo.toml | 44 +++--- pallets/fee-share/src/benchmarking.rs | 22 +-- pallets/fee-share/src/lib.rs | 194 +++++++++++++++----------- pallets/fee-share/src/tests.rs | 18 +-- 4 files changed, 156 insertions(+), 122 deletions(-) diff --git a/pallets/fee-share/Cargo.toml b/pallets/fee-share/Cargo.toml index 3cc8b8f6b..48bb407a4 100644 --- a/pallets/fee-share/Cargo.toml +++ b/pallets/fee-share/Cargo.toml @@ -8,41 +8,41 @@ edition = "2021" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -log = { workspace = true } -parity-scale-codec = { workspace = true, features = ["derive"] } -scale-info = { workspace = true, features = ["derive"] } +bifrost-primitives = { workspace = true } +bifrost-slp = { workspace = true } +bifrost-vtoken-minting = { workspace = true } +cumulus-primitives-core = { workspace = true } +frame-benchmarking = { workspace = true, optional = true } frame-support = { workspace = true } frame-system = { workspace = true } -frame-benchmarking = { workspace = true, optional = true } -bifrost-primitives = { workspace = true } -orml-traits = { workspace = true } -sp-std = { workspace = true } hex-literal = { workspace = true } +log = { workspace = true } +orml-traits = { workspace = true } pallet-balances = { workspace = true } +pallet-traits = { workspace = true } +parity-scale-codec = { workspace = true, features = ["derive"] } +scale-info = { workspace = true, features = ["derive"] } sp-arithmetic = { workspace = true } sp-core = { workspace = true } +sp-std = { workspace = true } xcm = { workspace = true } -bifrost-vtoken-minting = { workspace = true } zenlink-protocol = { workspace = true } -bifrost-slp = { workspace = true } -cumulus-primitives-core = { workspace = true } -pallet-traits = { workspace = true } [dev-dependencies] +bifrost-asset-registry = { workspace = true } +bifrost-currencies = { workspace = true } +env_logger = { workspace = true } +orml-oracle = { workspace = true } orml-tokens = { workspace = true } -orml-xtokens = { workspace = true } orml-traits = { workspace = true } -bifrost-currencies = { workspace = true } -xcm-executor = { workspace = true } -xcm-builder = { workspace = true } +orml-xtokens = { workspace = true } +pallet-prices = { workspace = true } pallet-xcm = { workspace = true } -sp-io = { workspace = true } sp-core = { workspace = true } +sp-io = { workspace = true } sp-runtime = { workspace = true } -bifrost-asset-registry = { workspace = true } -pallet-prices = { workspace = true } -orml-oracle = { workspace = true } -env_logger = { workspace = true } +xcm-builder = { workspace = true } +xcm-executor = { workspace = true } [features] default = ["std"] @@ -55,7 +55,7 @@ std = [ "bifrost-primitives/std", "orml-xtokens/std", "orml-traits/std", - "bifrost-vtoken-minting/std", + "bifrost-vtoken-minting/std", "zenlink-protocol/std", "bifrost-slp/std", "bifrost-asset-registry/std", @@ -65,6 +65,6 @@ std = [ runtime-benchmarks = [ "sp-runtime/runtime-benchmarks", - "frame-benchmarking/runtime-benchmarks" + "frame-benchmarking/runtime-benchmarks", ] try-runtime = ["frame-support/try-runtime"] diff --git a/pallets/fee-share/src/benchmarking.rs b/pallets/fee-share/src/benchmarking.rs index b86182f3e..c07911913 100644 --- a/pallets/fee-share/src/benchmarking.rs +++ b/pallets/fee-share/src/benchmarking.rs @@ -36,8 +36,8 @@ benchmarks! { const KSM: CurrencyId = CurrencyId::Token(TokenSymbol::KSM); let token_type = vec![KSM]; }: _(RawOrigin::Root, - token_type.clone(), - tokens_proportion.clone(), + BoundedVec::try_from(token_type.clone()).unwrap(), + BoundedVec::try_from(tokens_proportion.clone()).unwrap(), true) edit_distribution { @@ -47,14 +47,14 @@ benchmarks! { let token_type = vec![KSM]; assert_ok!(FeeShare::::create_distribution( RawOrigin::Root.into(), - vec![KSM], - tokens_proportion.clone(), + BoundedVec::try_from(vec![KSM]).unwrap(), + BoundedVec::try_from(tokens_proportion.clone()).unwrap(), true, )); }: _(RawOrigin::Root, 0, None, - Some(tokens_proportion.clone()), + Some(BoundedVec::try_from(tokens_proportion.clone()).unwrap()), Some(true)) set_era_length {}: _(RawOrigin::Root,BlockNumberFor::::from(10u32)) execute_distribute { @@ -64,8 +64,8 @@ benchmarks! { let token_type = vec![KSM]; assert_ok!(FeeShare::::create_distribution( RawOrigin::Root.into(), - vec![KSM], - tokens_proportion.clone(), + BoundedVec::try_from(vec![KSM]).unwrap(), + BoundedVec::try_from(tokens_proportion.clone()).unwrap(), true, )); }: _(RawOrigin::Root,0) @@ -76,8 +76,8 @@ benchmarks! { let token_type = vec![KSM]; assert_ok!(FeeShare::::create_distribution( RawOrigin::Root.into(), - vec![KSM], - tokens_proportion.clone(), + BoundedVec::try_from(vec![KSM]).unwrap(), + BoundedVec::try_from(tokens_proportion.clone()).unwrap(), true, )); }: _(RawOrigin::Root,0) @@ -88,8 +88,8 @@ benchmarks! { let token_type = vec![KSM]; assert_ok!(FeeShare::::create_distribution( RawOrigin::Root.into(), - vec![KSM], - tokens_proportion.clone(), + BoundedVec::try_from(vec![KSM]).unwrap(), + BoundedVec::try_from(tokens_proportion.clone()).unwrap(), true, )); }: _(RawOrigin::Root, diff --git a/pallets/fee-share/src/lib.rs b/pallets/fee-share/src/lib.rs index 058456213..7a5d4f670 100644 --- a/pallets/fee-share/src/lib.rs +++ b/pallets/fee-share/src/lib.rs @@ -45,7 +45,7 @@ use frame_system::pallet_prelude::*; use orml_traits::MultiCurrency; pub use pallet::*; use pallet_traits::PriceFeeder; -use sp_std::{cmp::Ordering, collections::btree_map::BTreeMap, vec::Vec}; +use sp_std::cmp::Ordering; pub use weights::WeightInfo; pub type AccountIdOf = ::AccountId; @@ -56,6 +56,31 @@ pub type CurrencyIdOf = <::MultiCurrency as MultiCurrency< type BalanceOf = <::MultiCurrency as MultiCurrency>>::Balance; +/// Distribution information +#[derive(Clone, Encode, Decode, PartialEq, Eq, RuntimeDebug, TypeInfo)] +pub struct Info { + /// Account id used for distribution + pub fee_share_account_id: AccountIdOf, + /// The token type of the distribution + pub token_type: BoundedVec>, + /// If the distribution is auto + pub if_auto: bool, +} + +/// USD Standard Accumulation Logic Configuration +#[derive(Clone, Encode, Decode, PartialEq, Eq, RuntimeDebug, TypeInfo)] +pub struct DollarStandardInfo { + /// The target value of the USD standard + pub target_value: u128, + /// The cumulative value of the USD standard + pub cumulative: u128, + /// The target account id of the USD standard + pub target_account_id: AccountIdOf, + /// Target block to perform accumulation clear operation + pub target_block: BlockNumberFor, + /// Cumulative clearing operation interval + pub interval: BlockNumberFor, +} #[frame_support::pallet] pub mod pallet { use super::*; @@ -85,24 +110,50 @@ pub mod pallet { #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { /// A successful call of the `CreateDistribution` extrinsic will create this event. - Created { distribution_id: DistributionId, info: Info> }, + Created { + /// Distribution ID + distribution_id: DistributionId, + /// Distribution information + info: Info>, + }, /// A successful call of the `EditDistribution` extrinsic will create this event. - Edited { distribution_id: DistributionId, info: Info> }, + Edited { + /// Distribution ID + distribution_id: DistributionId, + /// Distribution information + info: Info>, + }, /// A successful call of the `SetEraLength` extrinsic will create this event. - EraLengthSet { era_length: BlockNumberFor, next_era: BlockNumberFor }, + EraLengthSet { + /// The interval between distribution executions + era_length: BlockNumberFor, + /// The block number of the next era + next_era: BlockNumberFor, + }, /// A successful call of the `ExecuteDistribute` extrinsic will create this event. - Executed { distribution_id: DistributionId }, + Executed { + /// Distribution ID + distribution_id: DistributionId, + }, /// A successful call of the `DeleteDistribution` extrinsic will create this event. - Deleted { distribution_id: DistributionId }, + Deleted { + /// Distribution ID + distribution_id: DistributionId, + }, /// A failed call of the `ExecuteDistribute` extrinsic will create this event. ExecuteFailed { + /// Distribution ID distribution_id: DistributionId, + /// Distribution information info: Info>, + /// The block number of the next era next_era: BlockNumberFor, }, /// A successful call of the `SetUSDConfig` extrinsic will create this event. USDConfigSet { + /// Distribution ID distribution_id: DistributionId, + /// USD standard information info: DollarStandardInfo, AccountIdOf>, }, } @@ -123,24 +174,21 @@ pub mod pallet { IntervalIsZero, /// Value is zero ValueIsZero, + /// Tokens proportions not cleared + TokensProportionsNotCleared, } + /// The distribution information #[pallet::storage] pub type DistributionInfos = StorageMap<_, Twox64Concat, DistributionId, Info>>; - #[derive(Clone, Encode, Decode, PartialEq, Eq, RuntimeDebug, TypeInfo)] - pub struct Info { - /// The receiving address of the distribution - pub receiving_address: AccountIdOf, - /// The token type of the distribution - pub token_type: Vec, - /// The tokens proportion of the distribution - pub tokens_proportion: BTreeMap, - /// If the distribution is auto - pub if_auto: bool, - } + /// The proportion of the token distribution + #[pallet::storage] + pub type TokensProportions = + StorageDoubleMap<_, Twox64Concat, DistributionId, Twox64Concat, AccountIdOf, Perbill>; + /// USD Standard Accumulation Logic Configuration #[pallet::storage] pub type DollarStandardInfos = StorageMap< _, @@ -149,23 +197,11 @@ pub mod pallet { DollarStandardInfo, AccountIdOf>, >; - #[derive(Clone, Encode, Decode, PartialEq, Eq, RuntimeDebug, TypeInfo)] - pub struct DollarStandardInfo { - /// The target value of the USD standard - pub target_value: u128, - /// The cumulative value of the USD standard - pub cumulative: u128, - /// The target address of the USD standard - pub target_address: AccountIdOf, - /// Target block to perform accumulation clear operation - pub target_block: BlockNumberFor, - /// Cumulative clearing operation interval - pub interval: BlockNumberFor, - } - + /// The next distribution ID #[pallet::storage] pub type DistributionNextId = StorageValue<_, DistributionId, ValueQuery>; + /// The era length and the next era #[pallet::storage] pub type AutoEra = StorageValue<_, (BlockNumberFor, BlockNumberFor), ValueQuery>; @@ -174,7 +210,7 @@ pub mod pallet { impl Hooks> for Pallet { fn on_idle(bn: BlockNumberFor, _remaining_weight: Weight) -> Weight { DollarStandardInfos::::iter().for_each(|(distribution_id, mut info)| { - if bn == info.target_block { + if bn.eq(&info.target_block) { info.target_block = info.target_block.saturating_add(info.interval); info.cumulative = Zero::zero(); DollarStandardInfos::::insert(distribution_id, info); @@ -182,7 +218,7 @@ pub mod pallet { }); let (era_length, next_era) = AutoEra::::get(); if bn.eq(&next_era) { - for (distribution_id, info) in DistributionInfos::::iter() { + DistributionInfos::::iter().for_each(|(distribution_id, info)| { if info.if_auto { if let Some(e) = Self::execute_distribute_inner(distribution_id, &info).err() @@ -202,7 +238,7 @@ pub mod pallet { Self::deposit_event(Event::Executed { distribution_id }); } } - } + }); let next_era = next_era.saturating_add(era_length); AutoEra::::put((era_length, next_era)); } @@ -221,31 +257,23 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::create_distribution())] pub fn create_distribution( origin: OriginFor, - token_type: Vec, - tokens_proportion: Vec<(AccountIdOf, Perbill)>, + token_type: BoundedVec>, + tokens_proportion: BoundedVec<(AccountIdOf, Perbill), ConstU32<256>>, if_auto: bool, ) -> DispatchResult { T::ControlOrigin::ensure_origin(origin)?; + let distribution_id = DistributionNextId::::get(); let mut total_proportion = Perbill::from_percent(0); - let tokens_proportion_map: BTreeMap, Perbill> = tokens_proportion - .into_iter() - .map(|(k, v)| { - total_proportion = total_proportion.saturating_add(v); - (k, v) - }) - .collect(); + tokens_proportion.into_iter().for_each(|(k, v)| { + total_proportion = total_proportion.saturating_add(v); + TokensProportions::::insert(distribution_id, k, v); + }); ensure!(total_proportion.is_one(), Error::::NotSupportProportion); - let distribution_id = DistributionNextId::::get(); - let receiving_address = + let fee_share_account_id = T::FeeSharePalletId::get().into_sub_account_truncating(distribution_id); - let info = Info { - receiving_address, - token_type, - tokens_proportion: tokens_proportion_map, - if_auto, - }; + let info = Info { fee_share_account_id, token_type, if_auto }; DistributionInfos::::insert(distribution_id, info.clone()); DistributionNextId::::mutate(|id| -> DispatchResult { *id = id.checked_add(1).ok_or(ArithmeticError::Overflow)?; @@ -267,8 +295,8 @@ pub mod pallet { pub fn edit_distribution( origin: OriginFor, distribution_id: DistributionId, - token_type: Option>, - tokens_proportion: Option, Perbill)>>, + token_type: Option>>, + tokens_proportion: Option, Perbill), ConstU32<256>>>, if_auto: Option, ) -> DispatchResult { T::ControlOrigin::ensure_origin(origin)?; @@ -276,16 +304,17 @@ pub mod pallet { let mut info = DistributionInfos::::get(distribution_id) .ok_or(Error::::DistributionNotExist)?; if let Some(tokens_proportion) = tokens_proportion { + // Clear the original proportion + let res = + TokensProportions::::clear_prefix(distribution_id, u32::max_value(), None); + ensure!(res.maybe_cursor.is_none(), Error::::TokensProportionsNotCleared); + let mut total_proportion = Perbill::from_percent(0); - let tokens_proportion_map: BTreeMap, Perbill> = tokens_proportion - .into_iter() - .map(|(k, v)| { - total_proportion = total_proportion.saturating_add(v); - (k, v) - }) - .collect(); + tokens_proportion.into_iter().for_each(|(k, v)| { + total_proportion = total_proportion.saturating_add(v); + TokensProportions::::insert(distribution_id, k, v); + }); ensure!(total_proportion.is_one(), Error::::NotSupportProportion); - info.tokens_proportion = tokens_proportion_map; } if let Some(token_type) = token_type { @@ -365,8 +394,8 @@ pub mod pallet { /// - `distribution_id`: Distribution ID /// - `target_value`: Target's USD based value /// - `interval`: The interval of the cumulative clearing operation - /// - `target_address`: When the cumulative dollar value falls below the target_value, the - /// funds will be transferred to the target_address + /// - `target_account_id`: When the cumulative dollar value falls below the target_value, + /// the funds will be transferred to the target_account_id #[pallet::call_index(5)] #[pallet::weight(T::WeightInfo::set_usd_config())] pub fn set_usd_config( @@ -374,7 +403,7 @@ pub mod pallet { distribution_id: DistributionId, target_value: u128, interval: BlockNumberFor, - target_address: AccountIdOf, + target_account_id: AccountIdOf, ) -> DispatchResult { T::ControlOrigin::ensure_origin(origin)?; @@ -389,8 +418,8 @@ pub mod pallet { let info = DollarStandardInfo { target_value, cumulative: Zero::zero(), - target_address, - target_block: now + interval, + target_account_id, + target_block: now.saturating_add(interval), interval, }; DollarStandardInfos::::insert(distribution_id, info.clone()); @@ -408,7 +437,8 @@ pub mod pallet { let mut usd_value: FixedU128 = Zero::zero(); // Calculate the total value based on the US dollar standard infos.token_type.iter().try_for_each(|¤cy_id| -> DispatchResult { - let amount = T::MultiCurrency::free_balance(currency_id, &infos.receiving_address); + let amount = + T::MultiCurrency::free_balance(currency_id, &infos.fee_share_account_id); let value = Self::get_asset_value(currency_id, amount)?; usd_value = usd_value.checked_add(&value).ok_or(ArithmeticError::Overflow)?; Ok(()) @@ -426,16 +456,17 @@ pub mod pallet { .checked_add(usd_value.into_inner()) .ok_or(ArithmeticError::Overflow)?; DollarStandardInfos::::insert(distribution_id, &usd_infos); - return Self::transfer_all(infos, usd_infos.target_address); + return Self::transfer_all(infos, usd_infos.target_account_id); }, } } infos.token_type.iter().try_for_each(|¤cy_id| -> DispatchResult { let ed = T::MultiCurrency::minimum_balance(currency_id); - let amount = T::MultiCurrency::free_balance(currency_id, &infos.receiving_address); - infos.tokens_proportion.iter().try_for_each( - |(account_to_send, &proportion)| -> DispatchResult { + let amount = + T::MultiCurrency::free_balance(currency_id, &infos.fee_share_account_id); + TokensProportions::::iter_prefix(distribution_id).try_for_each( + |(account_to_send, proportion)| -> DispatchResult { let withdraw_amount = proportion.mul_floor(amount); if withdraw_amount < ed { let receiver_balance = @@ -445,12 +476,14 @@ pub mod pallet { .checked_add(&withdraw_amount) .ok_or(ArithmeticError::Overflow)?; if receiver_balance_after < ed { - Err(Error::::ExistentialDeposit)?; + // If the balance of the receiving account is less than the + // existential deposit, the balance is not transferred + return Ok(()); } } T::MultiCurrency::transfer( currency_id, - &infos.receiving_address, + &infos.fee_share_account_id, &account_to_send, withdraw_amount, ) @@ -462,12 +495,12 @@ pub mod pallet { pub fn get_price(currency_id: CurrencyIdOf) -> Result { let (price, _) = T::PriceFeeder::get_price(¤cy_id).ok_or(Error::::PriceOracleNotReady)?; - if price.is_zero() { - return Err(Error::::PriceIsZero.into()); - } log::trace!( target: "fee-share::get_price", "price: {:?}", price.into_inner() ); + if price.is_zero() { + return Err(Error::::PriceIsZero.into()); + } Ok(price) } @@ -485,14 +518,15 @@ pub mod pallet { fn transfer_all( infos: &Info>, - target_address: AccountIdOf, + target_account_id: AccountIdOf, ) -> DispatchResult { infos.token_type.iter().try_for_each(|¤cy_id| -> DispatchResult { - let amount = T::MultiCurrency::free_balance(currency_id, &infos.receiving_address); + let amount = + T::MultiCurrency::free_balance(currency_id, &infos.fee_share_account_id); T::MultiCurrency::transfer( currency_id, - &infos.receiving_address, - &target_address, + &infos.fee_share_account_id, + &target_account_id, amount, ) }) diff --git a/pallets/fee-share/src/tests.rs b/pallets/fee-share/src/tests.rs index 4b08089df..677e88227 100644 --- a/pallets/fee-share/src/tests.rs +++ b/pallets/fee-share/src/tests.rs @@ -31,8 +31,8 @@ fn on_idle() { assert_ok!(FeeShare::create_distribution( RuntimeOrigin::signed(ALICE), - vec![KSM], - tokens_proportion, + BoundedVec::try_from(vec![KSM]).unwrap(), + BoundedVec::try_from(tokens_proportion).unwrap(), true, )); let keeper: AccountId = @@ -53,15 +53,15 @@ fn edit_delete_distribution() { assert_ok!(FeeShare::create_distribution( RuntimeOrigin::signed(ALICE), - vec![KSM], - tokens_proportion.clone(), + BoundedVec::try_from(vec![KSM]).unwrap(), + BoundedVec::try_from(tokens_proportion.clone()).unwrap(), true, )); assert_ok!(FeeShare::edit_distribution( RuntimeOrigin::signed(ALICE), 0, None, // Some(vec![KSM]), - Some(tokens_proportion), + Some(BoundedVec::try_from(tokens_proportion).unwrap()), Some(false), )); let keeper: AccountId = @@ -90,8 +90,8 @@ fn set_usd_config_should_work() { assert_ok!(FeeShare::create_distribution( RuntimeOrigin::signed(ALICE), - vec![KSM], - tokens_proportion, + BoundedVec::try_from(vec![KSM]).unwrap(), + BoundedVec::try_from(tokens_proportion.clone()).unwrap(), true, )); @@ -145,8 +145,8 @@ fn set_usd_config_should_not_work() { ); assert_ok!(FeeShare::create_distribution( RuntimeOrigin::signed(ALICE), - vec![KSM], - tokens_proportion, + BoundedVec::try_from(vec![KSM]).unwrap(), + BoundedVec::try_from(tokens_proportion.clone()).unwrap(), true, )); assert_err!(