Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pallet_staking: Introduce Additional Account for Inflation Distribution Alongside PBR #2958

Closed
wants to merge 16 commits into from
Closed

pallet_staking: Introduce Additional Account for Inflation Distribution Alongside PBR #2958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
369d437
.
TarekkMA Sep 19, 2024
ecb2684
rename
TarekkMA Sep 19, 2024
fff0c7b
.
TarekkMA Sep 19, 2024
9b979fd
configs
TarekkMA Sep 19, 2024
0b93aef
fix tests
TarekkMA Sep 19, 2024
46d4173
fix brnches
TarekkMA Sep 19, 2024
15fcb08
fix weights
TarekkMA Sep 20, 2024
0e6b976
fix weights
TarekkMA Sep 20, 2024
db49f3f
fix test
TarekkMA Sep 20, 2024
557bfad
fix tests
TarekkMA Sep 20, 2024
408aa2d
formats
TarekkMA Sep 20, 2024
8010204
Trigger Build
TarekkMA Sep 20, 2024
17bd003
trigger build
TarekkMA Sep 20, 2024
de823b5
fix test
TarekkMA Sep 23, 2024
67cd0fd
fix weights
TarekkMA Sep 23, 2024
5664400
fix
TarekkMA Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 20 additions & 24 deletions pallets/parachain-staking/src/benchmarks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
//! Benchmarking
use crate::{
AwardedPts, BalanceOf, BottomDelegations, Call, CandidateBondLessRequest, Config,
DelegationAction, EnableMarkingOffline, Pallet, ParachainBondConfig, ParachainBondInfo, Points,
Range, RewardPayment, Round, ScheduledRequest, TopDelegations,
DelegationAction, EnableMarkingOffline, InflationDistributionConfig,
InflationDistributionConfigId, InflationDistributionInfo, Pallet, Points, Range, RewardPayment,
Round, ScheduledRequest, TopDelegations,
};
use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite};
use frame_support::traits::{Currency, Get, OnFinalize, OnInitialize};
Expand Down Expand Up @@ -276,17 +277,17 @@ benchmarks! {
assert_eq!(Pallet::<T>::inflation_config().annual, inflation_range);
}

set_parachain_bond_account {
let parachain_bond_account: T::AccountId = account("TEST", 0u32, USER_SEED);
}: _(RawOrigin::Root, parachain_bond_account.clone())
verify {
assert_eq!(Pallet::<T>::parachain_bond_info().account, parachain_bond_account);
}

set_parachain_bond_reserve_percent {
}: _(RawOrigin::Root, Percent::from_percent(33))
set_inflation_distribution_config {
let id = InflationDistributionConfigId::ParachainBondReserve;
let account: T::AccountId = account("TEST", 0u32, USER_SEED);
let percent = Percent::from_percent(33);
let config = InflationDistributionConfig {
account,
percent,
};
}: _(RawOrigin::Root, id.clone(), Some(config.clone()))
verify {
assert_eq!(Pallet::<T>::parachain_bond_info().percent, Percent::from_percent(33));
assert_eq!(Pallet::<T>::inflation_distribution_info(id), Some(config));
}

// ROOT DISPATCHABLES
Expand Down Expand Up @@ -1554,7 +1555,7 @@ benchmarks! {
let payout_round = round.current - reward_delay;
// may need:
// <Points<T>>
// <ParachainBondInfo<T>>
// <InflationDistributionInfo<T>>
// ensure parachain bond account exists so that deposit_into_existing succeeds
<Points<T>>::insert(payout_round, 100);

Expand All @@ -1564,10 +1565,12 @@ benchmarks! {
0,
min_candidate_stk::<T>(),
).0;
<ParachainBondInfo<T>>::put(ParachainBondConfig {
<InflationDistributionInfo<T>>::set(
InflationDistributionConfigId::ParachainBondReserve,
Some( InflationDistributionConfig {
account,
percent: Percent::from_percent(50),
});
}));

}: { Pallet::<T>::prepare_staking_payouts(round, current_slot); }
verify {
Expand Down Expand Up @@ -2332,16 +2335,9 @@ mod tests {
}

#[test]
fn bench_set_parachain_bond_account() {
new_test_ext().execute_with(|| {
assert_ok!(Pallet::<Test>::test_benchmark_set_parachain_bond_account());
});
}

#[test]
fn bench_set_parachain_bond_reserve_percent() {
fn bench_set_inflation_distribution_config() {
new_test_ext().execute_with(|| {
assert_ok!(Pallet::<Test>::test_benchmark_set_parachain_bond_reserve_percent());
assert_ok!(Pallet::<Test>::test_benchmark_set_inflation_distribution_config());
});
}

Expand Down
200 changes: 131 additions & 69 deletions pallets/parachain-staking/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub mod pallet {
use crate::{set::BoundedOrderedSet, traits::*, types::*, InflationInfo, Range, WeightInfo};
use crate::{AutoCompoundConfig, AutoCompoundDelegations};
use frame_support::fail;
use frame_support::pallet_prelude::*;
use frame_support::pallet_prelude::{ValueQuery, *};
use frame_support::traits::{
tokens::WithdrawReasons, Currency, Get, Imbalance, LockIdentifier, LockableCurrency,
ReservableCurrency,
Expand Down Expand Up @@ -226,6 +226,7 @@ pub mod pallet {
CannotSetBelowMin,
RoundLengthMustBeGreaterThanTotalSelectedCollators,
NoWritingSameValue,
TotalInflationDistributionPercentExceeds100,
TooLowCandidateCountWeightHintJoinCandidates,
TooLowCandidateCountWeightHintCancelLeaveCandidates,
TooLowCandidateCountToLeaveCandidates,
Expand Down Expand Up @@ -399,18 +400,18 @@ pub mod pallet {
account: T::AccountId,
rewards: BalanceOf<T>,
},
/// Transferred to account which holds funds reserved for parachain bond.
ReservedForParachainBond {
/// Inflation distributed to an account.
InflationDistributed {
id: InflationDistributionConfigId,
account: T::AccountId,
value: BalanceOf<T>,
},
/// Account (re)set for parachain bond treasury.
ParachainBondAccountSet {
old: T::AccountId,
new: T::AccountId,
/// Inflation distribution config updated.
InflationDistributionConfigUpdated {
id: InflationDistributionConfigId,
old: Option<InflationDistributionConfig<T::AccountId>>,
new: Option<InflationDistributionConfig<T::AccountId>>,
},
/// Percent of inflation reserved for parachain bond (re)set.
ParachainBondReservePercentSet { old: Percent, new: Percent },
/// Annual inflation input (first 3) was used to derive new per-round inflation (last 3)
InflationSet {
annual_min: Perbill,
Expand Down Expand Up @@ -518,10 +519,15 @@ pub mod pallet {
pub(crate) type TotalSelected<T: Config> = StorageValue<_, u32, ValueQuery>;

#[pallet::storage]
#[pallet::getter(fn parachain_bond_info)]
#[pallet::getter(fn inflation_distribution_info)]
/// Parachain bond config info { account, percent_of_inflation }
pub(crate) type ParachainBondInfo<T: Config> =
StorageValue<_, ParachainBondConfig<T::AccountId>, ValueQuery>;
pub(crate) type InflationDistributionInfo<T: Config> = StorageMap<
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's too complex and less efficient to use a StorageMap here.
Please go back to a StorageValue, and create instead a struct InflationDistributionConfig that contains all the field you need

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TarekkMA also, the StorageMap imply more reads at round change block, more complex code in staking pallet that is already too complex, and more complex and costly benchmarks.
We need to reduce the complexity of this pallet, as it was the place of many critical bugs in the past, It is very important not to add unnecessary complexity!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will work on the new impl in this PR: #2976

_,
Blake2_128Concat,
InflationDistributionConfigId,
InflationDistributionConfig<T::AccountId>,
OptionQuery,
>;

#[pallet::storage]
#[pallet::getter(fn round)]
Expand Down Expand Up @@ -787,12 +793,17 @@ pub mod pallet {
// Set collator commission to default config
<CollatorCommission<T>>::put(self.collator_commission);
// Set parachain bond config to default config
<ParachainBondInfo<T>>::put(ParachainBondConfig {
// must be set soon; if not => due inflation will be sent to collators/delegators
account: T::AccountId::decode(&mut sp_runtime::traits::TrailingZeroInput::zeroes())
<InflationDistributionInfo<T>>::insert(
InflationDistributionConfigId::ParachainBondReserve,
InflationDistributionConfig {
// must be set soon; if not => due inflation will be sent to collators/delegators
account: T::AccountId::decode(
&mut sp_runtime::traits::TrailingZeroInput::zeroes(),
)
.expect("infinite length input; no invalid inputs for type; qed"),
percent: self.parachain_bond_reserve_percent,
});
percent: self.parachain_bond_reserve_percent,
},
);
// Set total selected candidates to value from config
assert!(
self.num_selected_candidates >= T::MinSelectedCandidates::get(),
Expand Down Expand Up @@ -872,47 +883,47 @@ pub mod pallet {
Ok(().into())
}

/// Set the account that will hold funds set aside for parachain bond
#[pallet::call_index(2)]
#[pallet::weight(<T as Config>::WeightInfo::set_parachain_bond_account())]
pub fn set_parachain_bond_account(
origin: OriginFor<T>,
new: T::AccountId,
) -> DispatchResultWithPostInfo {
T::MonetaryGovernanceOrigin::ensure_origin(origin)?;
let ParachainBondConfig {
account: old,
percent,
} = <ParachainBondInfo<T>>::get();
ensure!(old != new, Error::<T>::NoWritingSameValue);
<ParachainBondInfo<T>>::put(ParachainBondConfig {
account: new.clone(),
percent,
});
Self::deposit_event(Event::ParachainBondAccountSet { old, new });
Ok(().into())
}

/// Set the percent of inflation set aside for parachain bond
#[pallet::call_index(3)]
#[pallet::weight(<T as Config>::WeightInfo::set_parachain_bond_reserve_percent())]
pub fn set_parachain_bond_reserve_percent(
origin: OriginFor<T>,
new: Percent,
) -> DispatchResultWithPostInfo {
T::MonetaryGovernanceOrigin::ensure_origin(origin)?;
let ParachainBondConfig {
account,
percent: old,
} = <ParachainBondInfo<T>>::get();
ensure!(old != new, Error::<T>::NoWritingSameValue);
<ParachainBondInfo<T>>::put(ParachainBondConfig {
account,
percent: new,
});
Self::deposit_event(Event::ParachainBondReservePercentSet { old, new });
Ok(().into())
}
// /// Set the account that will hold funds set aside for parachain bond
// #[pallet::call_index(2)]
// #[pallet::weight(<T as Config>::WeightInfo::set_parachain_bond_account())]
// pub fn set_parachain_bond_account(
// origin: OriginFor<T>,
// new: T::AccountId,
// ) -> DispatchResultWithPostInfo {
// T::MonetaryGovernanceOrigin::ensure_origin(origin)?;
// let InflationDistributionConfig {
// account: old,
// percent,
// } = <InflationDistributionInfo<T>>::get();
// ensure!(old != new, Error::<T>::NoWritingSameValue);
// <InflationDistributionInfo<T>>::put(InflationDistributionConfig {
// account: new.clone(),
// percent,
// });
// Self::deposit_event(Event::ParachainBondAccountSet { old, new });
// Ok(().into())
// }

// /// Set the percent of inflation set aside for parachain bond
// #[pallet::call_index(3)]
// #[pallet::weight(<T as Config>::WeightInfo::set_parachain_bond_reserve_percent())]
// pub fn set_parachain_bond_reserve_percent(
// origin: OriginFor<T>,
// new: Percent,
// ) -> DispatchResultWithPostInfo {
// T::MonetaryGovernanceOrigin::ensure_origin(origin)?;
// let InflationDistributionConfig {
// account,
// percent: old,
// } = <InflationDistributionInfo<T>>::get();
// ensure!(old != new, Error::<T>::NoWritingSameValue);
// <InflationDistributionInfo<T>>::put(InflationDistributionConfig {
// account,
// percent: new,
// });
// Self::deposit_event(Event::ParachainBondReservePercentSet { old, new });
// Ok(().into())
// }

/// Set the total number of collator candidates selected per round
/// - changes are not applied until the start of the next round
Expand Down Expand Up @@ -1465,6 +1476,55 @@ pub mod pallet {
T::MonetaryGovernanceOrigin::ensure_origin(origin.clone())?;
Self::join_candidates_inner(account, bond, candidate_count)
}

/// Set or remove the inflation distribution configuration, which specifies how inflation
/// is distributed between the parachain bond account and other accounts.
/// The total percentage of all inflation distributions must not exceed 100%.
///
/// To remove the configuration for a given `id`, pass `None` as the `config`.
///
/// # Arguments
/// * `id` - The identifier of the inflation distribution configuration.
/// * `config` - The new configuration, or `None` to remove the current one.
///
/// # Errors
/// * `NoWritingSameValue` - Occurs if the provided configuration is the same as the existing one.
/// * `TotalInflationDistributionPercentExceeds100` - Occurs if the total inflation distribution
/// across all configurations exceeds 100% after applying the new configuration.
#[pallet::call_index(32)]
#[pallet::weight(<T as Config>::WeightInfo::set_inflation_distribution_config())]
pub fn set_inflation_distribution_config(
origin: OriginFor<T>,
id: InflationDistributionConfigId,
config: Option<InflationDistributionConfig<T::AccountId>>,
) -> DispatchResultWithPostInfo {
T::MonetaryGovernanceOrigin::ensure_origin(origin)?;
let old_config = <InflationDistributionInfo<T>>::get(id.clone());
ensure!(old_config != config, Error::<T>::NoWritingSameValue);

let mut total_percent = Percent::zero();
for config in <InflationDistributionInfo<T>>::iter_values() {
total_percent = total_percent.saturating_add(config.percent);
}
if let Some(ref old_config) = old_config {
total_percent = total_percent.saturating_add(old_config.percent);
}
if let Some(ref config) = config {
total_percent = total_percent.saturating_add(config.percent);
}
ensure!(
total_percent <= Percent::from_percent(100),
Error::<T>::TotalInflationDistributionPercentExceeds100
);

<InflationDistributionInfo<T>>::set(id.clone(), config.clone());
Self::deposit_event(Event::InflationDistributionConfigUpdated {
id,
old: old_config,
new: config,
});
Ok(().into())
}
}

/// Represents a payout made via `pay_one_collator_reward`.
Expand Down Expand Up @@ -1839,17 +1899,19 @@ pub mod pallet {

// reserve portion of issuance for parachain bond account
let mut left_issuance = total_issuance;
let bond_config = <ParachainBondInfo<T>>::get();
let parachain_bond_reserve = bond_config.percent * total_issuance;
if let Ok(imb) =
T::Currency::deposit_into_existing(&bond_config.account, parachain_bond_reserve)
{
// update round issuance if transfer succeeds
left_issuance = left_issuance.saturating_sub(imb.peek());
Self::deposit_event(Event::ReservedForParachainBond {
account: bond_config.account,
value: imb.peek(),
});
let distribution_configs = <InflationDistributionInfo<T>>::iter().collect::<Vec<_>>();

for (id, config) in distribution_configs {
let reserve = config.percent * total_issuance;
if let Ok(imb) = T::Currency::deposit_into_existing(&config.account, reserve) {
// update round issuance if transfer succeeds
left_issuance = left_issuance.saturating_sub(imb.peek());
Self::deposit_event(Event::InflationDistributed {
id,
account: config.account,
value: imb.peek(),
});
}
}

let payout = DelayedPayout {
Expand Down
Loading
Loading