diff --git a/substrate/srml/staking/src/lib.rs b/substrate/srml/staking/src/lib.rs index 11aa4f3c7d..d1d690f75f 100644 --- a/substrate/srml/staking/src/lib.rs +++ b/substrate/srml/staking/src/lib.rs @@ -175,7 +175,8 @@ //! //! Total reward is split among validators and their nominators depending on the number of points //! they received during the era. Points are added to a validator using -//! [`add_reward_points_to_validator`](./enum.Call.html#variant.add_reward_points_to_validator). +//! [`reward_by_ids`](./enum.Call.html#variant.reward_by_ids) or +//! [`reward_by_indices`](./enum.Call.html#variant.reward_by_indices). //! //! [`Module`](./struct.Module.html) implements //! [`authorship::EventHandler`](../srml_authorship/trait.EventHandler.html) to add reward points @@ -325,6 +326,18 @@ pub struct EraRewards { rewards: Vec, } +impl EraRewards { + /// Add the reward to the validator at the given index. Index must be valid + /// (i.e. `index < current_elected.len()`). + fn add_points_to_index(&mut self, index: u32, points: u32) { + if let Some(new_total) = self.total.checked_add(points) { + self.total = new_total; + self.rewards.resize((index as usize + 1).max(self.rewards.len()), 0); + self.rewards[index as usize] += points; // Addition is less than total + } + } +} + /// Indicates the initial status of the staker. #[cfg_attr(feature = "std", derive(Debug, Serialize, Deserialize))] pub enum StakerStatus { @@ -1399,22 +1412,46 @@ impl Module { } } - /// Add reward points to validator. + /// Add reward points to validators using their stash account ID. + /// + /// Validators are keyed by stash account ID and must be in the current elected set. + /// + /// For each element in the iterator the given number of points in u32 is added to the + /// validator, thus duplicates are handled. /// /// At the end of the era each the total payout will be distributed among validator /// relatively to their points. - pub fn add_reward_points_to_validator(validator: T::AccountId, points: u32) { - >::current_elected().iter() - .position(|elected| *elected == validator) - .map(|index| { - CurrentEraRewards::mutate(|rewards| { - if let Some(new_total) = rewards.total.checked_add(points) { - rewards.total = new_total; - rewards.rewards.resize((index + 1).max(rewards.rewards.len()), 0); - rewards.rewards[index] += points; // Addition is less than total - } - }); - }); + /// + /// COMPLEXITY: Complexity is `number_of_validator_to_reward x current_elected_len`. + /// If you need to reward lots of validator consider using `reward_by_indices`. + pub fn reward_by_ids(validators_points: impl IntoIterator) { + CurrentEraRewards::mutate(|rewards| { + let current_elected = >::current_elected(); + for (validator, points) in validators_points.into_iter() { + if let Some(index) = current_elected.iter() + .position(|elected| *elected == validator) + { + rewards.add_points_to_index(index as u32, points); + } + } + }); + } + + /// Add reward points to validators using their validator index. + /// + /// For each element in the iterator the given number of points in u32 is added to the + /// validator, thus duplicates are handled. + pub fn reward_by_indices(validators_points: impl IntoIterator) { + // TODO: This can be optimised once #3302 is implemented. + let current_elected_len = >::current_elected().len() as u32; + + CurrentEraRewards::mutate(|rewards| { + for (validator_index, points) in validators_points.into_iter() { + if validator_index < current_elected_len { + rewards.add_points_to_index(validator_index, points); + } + } + }); } } @@ -1444,11 +1481,13 @@ impl OnFreeBalanceZero for Module { /// * 1 point to the producer of each referenced uncle block. impl authorship::EventHandler for Module { fn note_author(author: T::AccountId) { - Self::add_reward_points_to_validator(author, 20); + Self::reward_by_ids(vec![(author, 20)]); } fn note_uncle(author: T::AccountId, _age: T::BlockNumber) { - Self::add_reward_points_to_validator(>::author(), 2); - Self::add_reward_points_to_validator(author, 1); + Self::reward_by_ids(vec![ + (>::author(), 2), + (author, 1) + ]) } } diff --git a/substrate/srml/staking/src/mock.rs b/substrate/srml/staking/src/mock.rs index 787c4f13b7..a281d818a2 100644 --- a/substrate/srml/staking/src/mock.rs +++ b/substrate/srml/staking/src/mock.rs @@ -422,10 +422,12 @@ pub fn current_total_payout_for_duration(duration: u64) -> u64 { res } -pub fn add_reward_points_to_all_elected() { - for v in >::current_elected() { - >::add_reward_points_to_validator(v, 1); - } +pub fn reward_all_elected() { + let rewards = >::current_elected().iter() + .map(|v| (*v, 1)) + .collect::>(); + + >::reward_by_ids(rewards) } pub fn validator_controllers() -> Vec { diff --git a/substrate/srml/staking/src/tests.rs b/substrate/srml/staking/src/tests.rs index 96c8b9d5f2..cf0ef31526 100644 --- a/substrate/srml/staking/src/tests.rs +++ b/substrate/srml/staking/src/tests.rs @@ -357,12 +357,12 @@ fn rewards_should_work() { Session::on_initialize(System::block_number()); assert_eq!(Staking::current_era(), 0); assert_eq!(Session::current_index(), 1); - >::add_reward_points_to_validator(11, 50); - >::add_reward_points_to_validator(11, 50); + >::reward_by_ids(vec![(11, 50)]); + >::reward_by_ids(vec![(11, 50)]); // This is the second validator of the current elected set. - >::add_reward_points_to_validator(21, 50); + >::reward_by_ids(vec![(21, 50)]); // This must be no-op as it is not an elected validator. - >::add_reward_points_to_validator(1001, 10_000); + >::reward_by_ids(vec![(1001, 10_000)]); // Compute total payout now for whole duration as other parameter won't change let total_payout = current_total_payout_for_duration(9 * 5); @@ -412,7 +412,7 @@ fn multi_era_reward_should_work() { let total_payout_0 = current_total_payout_for_duration(3); assert!(total_payout_0 > 10); // Test is meaningfull if reward something dbg!(>::slot_stake()); - >::add_reward_points_to_validator(11, 1); + >::reward_by_ids(vec![(11, 1)]); start_session(0); start_session(1); @@ -426,7 +426,7 @@ fn multi_era_reward_should_work() { let total_payout_1 = current_total_payout_for_duration(3); assert!(total_payout_1 > 10); // Test is meaningfull if reward something - >::add_reward_points_to_validator(11, 101); + >::reward_by_ids(vec![(11, 101)]); // new era is triggered here. start_session(5); @@ -631,10 +631,10 @@ fn nominating_and_rewards_should_work() { // the total reward for era 0 let total_payout_0 = current_total_payout_for_duration(3); assert!(total_payout_0 > 100); // Test is meaningfull if reward something - >::add_reward_points_to_validator(41, 1); - >::add_reward_points_to_validator(31, 1); - >::add_reward_points_to_validator(21, 10); // must be no-op - >::add_reward_points_to_validator(11, 10); // must be no-op + >::reward_by_ids(vec![(41, 1)]); + >::reward_by_ids(vec![(31, 1)]); + >::reward_by_ids(vec![(21, 10)]); // must be no-op + >::reward_by_ids(vec![(11, 10)]); // must be no-op start_era(1); @@ -706,10 +706,10 @@ fn nominating_and_rewards_should_work() { // the total reward for era 1 let total_payout_1 = current_total_payout_for_duration(3); assert!(total_payout_1 > 100); // Test is meaningfull if reward something - >::add_reward_points_to_validator(41, 10); // must be no-op - >::add_reward_points_to_validator(31, 10); // must be no-op - >::add_reward_points_to_validator(21, 2); - >::add_reward_points_to_validator(11, 1); + >::reward_by_ids(vec![(41, 10)]); // must be no-op + >::reward_by_ids(vec![(31, 10)]); // must be no-op + >::reward_by_ids(vec![(21, 2)]); + >::reward_by_ids(vec![(11, 1)]); start_era(2); @@ -772,7 +772,7 @@ fn nominators_also_get_slashed() { let total_payout = current_total_payout_for_duration(3); assert!(total_payout > 100); // Test is meaningfull if reward something - >::add_reward_points_to_validator(11, 1); + >::reward_by_ids(vec![(11, 1)]); // new era, pay rewards, start_era(1); @@ -967,7 +967,7 @@ fn reward_destination_works() { // Compute total payout now for whole duration as other parameter won't change let total_payout_0 = current_total_payout_for_duration(3); assert!(total_payout_0 > 100); // Test is meaningfull if reward something - >::add_reward_points_to_validator(11, 1); + >::reward_by_ids(vec![(11, 1)]); start_era(1); @@ -989,7 +989,7 @@ fn reward_destination_works() { // Compute total payout now for whole duration as other parameter won't change let total_payout_1 = current_total_payout_for_duration(3); assert!(total_payout_1 > 100); // Test is meaningfull if reward something - >::add_reward_points_to_validator(11, 1); + >::reward_by_ids(vec![(11, 1)]); start_era(2); @@ -1016,7 +1016,7 @@ fn reward_destination_works() { // Compute total payout now for whole duration as other parameter won't change let total_payout_2 = current_total_payout_for_duration(3); assert!(total_payout_2 > 100); // Test is meaningfull if reward something - >::add_reward_points_to_validator(11, 1); + >::reward_by_ids(vec![(11, 1)]); start_era(3); @@ -1070,7 +1070,7 @@ fn validator_payment_prefs_work() { // Compute total payout now for whole duration as other parameter won't change let total_payout_0 = current_total_payout_for_duration(3); assert!(total_payout_0 > 100); // Test is meaningfull if reward something - >::add_reward_points_to_validator(11, 1); + >::reward_by_ids(vec![(11, 1)]); start_era(1); @@ -1284,8 +1284,8 @@ fn slot_stake_is_least_staked_validator_and_exposure_defines_maximum_punishment( // Compute total payout now for whole duration as other parameter won't change let total_payout_0 = current_total_payout_for_duration(3); assert!(total_payout_0 > 100); // Test is meaningfull if reward something - >::add_reward_points_to_validator(11, 1); - >::add_reward_points_to_validator(21, 1); + >::reward_by_ids(vec![(11, 1)]); + >::reward_by_ids(vec![(21, 1)]); // New era --> rewards are paid --> stakes are changed start_era(1); @@ -1777,7 +1777,7 @@ fn bond_with_little_staked_value_bounded_by_slot_stake() { let total_payout_0 = current_total_payout_for_duration(3); assert!(total_payout_0 > 100); // Test is meaningfull if reward something - add_reward_points_to_all_elected(); + reward_all_elected(); start_era(1); // 2 is elected. @@ -1792,7 +1792,7 @@ fn bond_with_little_staked_value_bounded_by_slot_stake() { let total_payout_1 = current_total_payout_for_duration(3); assert!(total_payout_1 > 100); // Test is meaningfull if reward something - add_reward_points_to_all_elected(); + reward_all_elected(); start_era(2); assert_eq_uvec!(validator_controllers(), vec![20, 10, 2]); @@ -2090,14 +2090,44 @@ fn reward_from_authorship_event_handler_works() { // An uncle author that is not currently elected doesn't get rewards, // but the block producer does get reward for referencing it. >::note_uncle(31, 1); + // Rewarding the same two times works. + >::note_uncle(11, 1); // Not mandatory but must be coherent with rewards assert_eq!(>::get(), vec![21, 11]); - // 21 is rewarded as an uncle procuder - // 11 is rewarded as a block procuder and unclde referencer - assert_eq!(CurrentEraRewards::get().rewards, vec![1, 20+2*2]); - assert_eq!(CurrentEraRewards::get().total, 25); + // 21 is rewarded as an uncle producer + // 11 is rewarded as a block procuder and uncle referencer and uncle producer + assert_eq!(CurrentEraRewards::get().rewards, vec![1, 20+2*3 + 1]); + assert_eq!(CurrentEraRewards::get().total, 28); + }) +} + +#[test] +fn add_reward_points_fns_works() { + with_externalities(&mut ExtBuilder::default() + .build(), + || { + let validators = >::current_elected(); + // Not mandatory but must be coherent with rewards + assert_eq!(validators, vec![21, 11]); + + >::reward_by_indices(vec![ + (0, 1), + (1, 1), + (2, 1), + (1, 1), + ]); + + >::reward_by_ids(vec![ + (21, 1), + (11, 1), + (31, 1), + (11, 1), + ]); + + assert_eq!(CurrentEraRewards::get().rewards, vec![2, 4]); + assert_eq!(CurrentEraRewards::get().total, 6); }) }