-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
Sys 4010 make autogrowth mechanism configurable #422
Changes from 5 commits
645a831
064d562
d7e6b7b
6768c29
96e983a
324da7c
7fe7233
0d0fdeb
8f96931
1b547c4
cbc245f
741b1ad
1c4f53f
19eadc2
f75be36
d69048e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -245,6 +245,8 @@ pub mod pallet { | |||||||
type AccountToBytesConvert: pallet_avn::AccountToBytesConverter<Self::AccountId>; | ||||||||
|
||||||||
type BridgeInterface: pallet_avn::BridgeInterface; | ||||||||
|
||||||||
type GrowthEnabled: Get<bool>; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
} | ||||||||
|
||||||||
#[pallet::error] | ||||||||
|
@@ -489,6 +491,10 @@ pub mod pallet { | |||||||
} | ||||||||
} | ||||||||
|
||||||||
#[pallet::storage] | ||||||||
#[pallet::getter(fn growth_enabled)] | ||||||||
pub type GrowthEnabled<T> = StorageValue<_, bool, ValueQuery>; | ||||||||
|
||||||||
#[pallet::storage] | ||||||||
#[pallet::getter(fn delay)] | ||||||||
/// Number of eras to wait before executing any staking action | ||||||||
|
@@ -687,6 +693,7 @@ pub mod pallet { | |||||||
pub delay: EraIndex, | ||||||||
pub min_collator_stake: BalanceOf<T>, | ||||||||
pub min_total_nominator_stake: BalanceOf<T>, | ||||||||
pub growth_enabled: bool, | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this needed? |
||||||||
} | ||||||||
|
||||||||
impl<T: Config> Default for GenesisConfig<T> { | ||||||||
|
@@ -697,6 +704,7 @@ pub mod pallet { | |||||||
delay: Default::default(), | ||||||||
min_collator_stake: Default::default(), | ||||||||
min_total_nominator_stake: Default::default(), | ||||||||
growth_enabled: T::GrowthEnabled::get(), | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||||||||
} | ||||||||
} | ||||||||
} | ||||||||
|
@@ -760,6 +768,8 @@ pub mod pallet { | |||||||
assert!(self.delay > 0, "Delay must be greater than 0."); | ||||||||
<Delay<T>>::put(self.delay); | ||||||||
|
||||||||
GrowthEnabled::<T>::put(self.growth_enabled); | ||||||||
|
||||||||
// Set min staking values | ||||||||
<MinCollatorStake<T>>::put(self.min_collator_stake); | ||||||||
<MinTotalNominatorStake<T>>::put(self.min_total_nominator_stake); | ||||||||
|
@@ -1675,6 +1685,7 @@ pub mod pallet { | |||||||
AdminSettings::Delay(d) => <Delay<T>>::put(d), | ||||||||
AdminSettings::MinCollatorStake(s) => <MinCollatorStake<T>>::put(s), | ||||||||
AdminSettings::MinTotalNominatorStake(s) => <MinTotalNominatorStake<T>>::put(s), | ||||||||
AdminSettings::GrowthEnabled(b) => <GrowthEnabled<T>>::put(b), | ||||||||
} | ||||||||
|
||||||||
Self::deposit_event(Event::AdminSettingsUpdated { value }); | ||||||||
|
@@ -1831,18 +1842,21 @@ pub mod pallet { | |||||||
|
||||||||
<DelayedPayouts<T>>::insert(era_to_payout, &payout); | ||||||||
|
||||||||
let collator_scores_vec: Vec<CollatorScore<T::AccountId>> = | ||||||||
<AwardedPts<T>>::iter_prefix(era_to_payout) | ||||||||
.map(|(collator, points)| CollatorScore::new(collator, points)) | ||||||||
.collect::<Vec<CollatorScore<T::AccountId>>>(); | ||||||||
let collator_scores = BoundedVec::truncate_from(collator_scores_vec); | ||||||||
Self::update_collator_payout( | ||||||||
era_to_payout, | ||||||||
total_staked, | ||||||||
payout, | ||||||||
total_points, | ||||||||
collator_scores, | ||||||||
); | ||||||||
let growth_enabled = GrowthEnabled::<T>::get(); | ||||||||
if growth_enabled { | ||||||||
let collator_scores_vec: Vec<CollatorScore<T::AccountId>> = | ||||||||
<AwardedPts<T>>::iter_prefix(era_to_payout) | ||||||||
.map(|(collator, points)| CollatorScore::new(collator, points)) | ||||||||
.collect::<Vec<CollatorScore<T::AccountId>>>(); | ||||||||
let collator_scores = BoundedVec::truncate_from(collator_scores_vec); | ||||||||
Self::update_collator_payout( | ||||||||
era_to_payout, | ||||||||
total_staked, | ||||||||
payout, | ||||||||
total_points, | ||||||||
collator_scores, | ||||||||
); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
/// Wrapper around pay_one_collator_reward which handles the following logic: | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -203,6 +203,7 @@ parameter_types! { | |
pub const ErasPerGrowthPeriod: u32 = 2; | ||
pub const RewardPotId: PalletId = PalletId(*b"av/vamgr"); | ||
pub const MaxCandidates:u32 = 100; | ||
pub const GrowthEnabled: bool = true; | ||
} | ||
|
||
pub struct IsRegistered; | ||
|
@@ -242,6 +243,7 @@ impl Config for Test { | |
type MaxCandidates = MaxCandidates; | ||
type AccountToBytesConvert = AVN; | ||
type BridgeInterface = EthBridge; | ||
type GrowthEnabled = GrowthEnabled; | ||
} | ||
|
||
// Deal with any positive imbalance by sending it to the fake treasury | ||
|
@@ -566,6 +568,7 @@ impl ExtBuilder { | |
delay: 2, | ||
min_collator_stake: self.min_collator_stake, | ||
min_total_nominator_stake: self.min_total_nominator_stake, | ||
growth_enabled: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
} | ||
.assimilate_storage(&mut t) | ||
.expect("Parachain Staking's storage can be assimilated"); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
use crate::{ | ||
assert_event_emitted, | ||
assert_event_emitted, assert_last_event, | ||
mock::{ | ||
get_default_block_per_era, roll_one_block, roll_to_era_begin, set_author, set_reward_pot, | ||
AccountId, Balances, ErasPerGrowthPeriod, ExtBuilder, ParachainStaking, RewardPaymentDelay, | ||
RuntimeOrigin, System, Test, TestAccount, | ||
RuntimeEvent, RuntimeOrigin, System, Test, TestAccount, | ||
}, | ||
BalanceOf, CollatorScore, EraIndex, Error, Event, Growth, GrowthInfo, GrowthPeriod, | ||
GrowthPeriodInfo, ProcessedGrowthPeriods, | ||
AdminSettings, BalanceOf, CollatorScore, EraIndex, Error, Event, Growth, GrowthEnabled, | ||
GrowthInfo, GrowthPeriod, GrowthPeriodInfo, ProcessedGrowthPeriods, | ||
}; | ||
use codec::{Decode, Encode}; | ||
use frame_support::{assert_noop, assert_ok}; | ||
|
@@ -67,6 +67,19 @@ fn increase_collator_nomination( | |
)); | ||
} | ||
|
||
fn disable_growth() { | ||
let new_growth_enabled_setting = AdminSettings::<BalanceOf<Test>>::GrowthEnabled(false); | ||
assert_ok!(ParachainStaking::set_admin_setting( | ||
RuntimeOrigin::root(), | ||
new_growth_enabled_setting.clone() | ||
)); | ||
|
||
assert_eq!(<GrowthEnabled<Test>>::get(), false); | ||
assert_last_event!(RuntimeEvent::ParachainStaking(Event::AdminSettingsUpdated { | ||
value: new_growth_enabled_setting | ||
})); | ||
} | ||
|
||
fn get_expected_block_number(growth_index: u64) -> u64 { | ||
return get_default_block_per_era() as u64 * ErasPerGrowthPeriod::get() as u64 * growth_index | ||
} | ||
|
@@ -200,6 +213,68 @@ fn growth_period_indices_updated_correctly() { | |
}); | ||
} | ||
|
||
mod growth_disabled { | ||
use super::*; | ||
|
||
#[test] | ||
fn growth_period_indices_are_not_updated() { | ||
let collator_1 = to_acc_id(1u64); | ||
let collator_2 = to_acc_id(2u64); | ||
ExtBuilder::default() | ||
.with_balances(vec![(collator_1, 100), (collator_2, 100)]) | ||
.with_candidates(vec![(collator_1, 10), (collator_2, 10)]) | ||
.build() | ||
.execute_with(|| { | ||
disable_growth(); | ||
let initial_growth_period_index = 0; | ||
let mut era_index = ParachainStaking::era().current; | ||
|
||
set_author(era_index, collator_1, 5); | ||
set_author(era_index, collator_2, 5); | ||
era_index = roll_one_growth_period(era_index); | ||
|
||
let growth_period_info = ParachainStaking::growth_period_info(); | ||
// ensure indice is not updated | ||
assert_eq!(initial_growth_period_index, growth_period_info.start_era_index); | ||
}); | ||
} | ||
|
||
#[test] | ||
fn growth_info_is_not_updated() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
let collator_1 = to_acc_id(1u64); | ||
let collator_2 = to_acc_id(2u64); | ||
let collator1_stake = 20; | ||
let collator2_stake = 10; | ||
let reward_payment_delay = RewardPaymentDelay::get(); | ||
ExtBuilder::default() | ||
.with_balances(vec![(collator_1, 10000), (collator_2, 10000)]) | ||
.with_candidates(vec![(collator_1, collator1_stake), (collator_2, collator2_stake)]) | ||
.build() | ||
.execute_with(|| { | ||
disable_growth(); | ||
let num_era_to_roll_foreward = reward_payment_delay + 1; | ||
|
||
// Setup data by rolling forward and letting the system generate staking rewards. | ||
// This is not "faked" data. | ||
let raw_era_data: HashMap<EraIndex, GrowthData> = roll_foreward_and_pay_stakers( | ||
num_era_to_roll_foreward, | ||
collator_1, | ||
collator_2, | ||
collator1_stake, | ||
collator2_stake, | ||
); | ||
|
||
let growth = ParachainStaking::growth(1); | ||
// Ensure info is not updated | ||
assert_eq!(growth.number_of_accumulations, 0); | ||
assert_eq!(growth.total_points, 0); | ||
assert_eq!(growth.total_stake_accumulated, 0); | ||
assert_eq!(growth.total_staker_reward, 0); | ||
assert_eq!(growth.collator_scores.len(), 0); | ||
}); | ||
} | ||
} | ||
|
||
mod growth_info_recorded_correctly { | ||
use super::*; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -262,6 +262,7 @@ parameter_types! { | |
pub const RewardPaymentDelay: u32 = 2; | ||
pub const RewardPotId: PalletId = PalletId(*b"av/vamgr"); | ||
pub const MaxCandidates: u32 = 100; | ||
pub const GrowthEnabled: bool = true; | ||
} | ||
|
||
impl parachain_staking::Config for TestRuntime { | ||
|
@@ -286,6 +287,7 @@ impl parachain_staking::Config for TestRuntime { | |
type MaxCandidates = MaxCandidates; | ||
type AccountToBytesConvert = AVN; | ||
type BridgeInterface = EthBridge; | ||
type GrowthEnabled = GrowthEnabled; | ||
} | ||
|
||
impl pallet_session::historical::Config for TestRuntime { | ||
|
@@ -439,6 +441,7 @@ impl ExtBuilder { | |
delay: 2, | ||
min_collator_stake: 10, | ||
min_total_nominator_stake: 5, | ||
growth_enabled: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and here |
||
} | ||
.assimilate_storage(&mut self.storage); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -296,6 +296,7 @@ parameter_types! { | |
pub const RewardPaymentDelay: u32 = 2; | ||
pub const RewardPotId: PalletId = PalletId(*b"av/vamgr"); | ||
pub const MaxCandidates: u32 = 256; | ||
pub const GrowthEnabled: bool = true; | ||
} | ||
|
||
impl parachain_staking::Config for TestRuntime { | ||
|
@@ -320,6 +321,7 @@ impl parachain_staking::Config for TestRuntime { | |
type MaxCandidates = MaxCandidates; | ||
type AccountToBytesConvert = AVN; | ||
type BridgeInterface = EthBridge; | ||
type GrowthEnabled = GrowthEnabled; | ||
} | ||
|
||
/// An extrinsic type used for tests. | ||
|
@@ -502,6 +504,7 @@ impl ExtBuilder { | |
delay: 2, | ||
min_collator_stake: 10, | ||
min_total_nominator_stake: 5, | ||
growth_enabled: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here too |
||
} | ||
.assimilate_storage(&mut self.storage); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we are setting it in storage, I guess we can remove the config trait?