diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index ca3fb51ecb74919fe775fd320fd61bb132f4c7b8..489070dca0260bd70a5d1d7e6a044a4bd596db25 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -2338,6 +2338,9 @@ impl<T: Trait> Module<T> { // candidates. let snapshot_length = <SnapshotValidators<T>>::decode_len() .map_err(|_| Error::<T>::SnapshotUnavailable)?; + + // check the winner length only here and when we know the length of the snapshot validators + // length. let desired_winners = Self::validator_count().min(snapshot_length as u32); ensure!(winners.len() as u32 == desired_winners, Error::<T>::PhragmenBogusWinnerCount); diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index a34f3425564cdd1b3cf4ec3ceb3194c305540e2e..21400b0b8d4c2fe5c1bb30810f6b6fe58518e0f9 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -834,7 +834,7 @@ pub(crate) fn horrible_phragmen_with_post_processing( // Ensure that this result is worse than seq-phragmen. Otherwise, it should not have been used // for testing. let score = { - let (_, _, better_score) = prepare_submission_with(true, |_| {}); + let (_, _, better_score) = prepare_submission_with(true, 0, |_| {}); let support = build_support_map::<AccountId>(&winners, &staked_assignment).0; let score = evaluate_support(&support); @@ -875,6 +875,7 @@ pub(crate) fn horrible_phragmen_with_post_processing( // cannot do it since we want to have `tweak` injected into the process. pub(crate) fn prepare_submission_with( do_reduce: bool, + iterations: usize, tweak: impl FnOnce(&mut Vec<StakedAssignment<AccountId>>), ) -> (CompactAssignments, Vec<ValidatorIndex>, PhragmenScore) { // run phragmen on the default stuff. @@ -882,14 +883,25 @@ pub(crate) fn prepare_submission_with( winners, assignments, } = Staking::do_phragmen::<OffchainAccuracy>().unwrap(); - let winners = winners.into_iter().map(|(w, _)| w).collect::<Vec<AccountId>>(); + let winners = sp_phragmen::to_without_backing(winners); let stake_of = |who: &AccountId| -> VoteWeight { <CurrencyToVoteHandler as Convert<Balance, VoteWeight>>::convert( Staking::slashable_balance_of(&who) ) }; + let mut staked = sp_phragmen::assignment_ratio_to_staked(assignments, stake_of); + let (mut support_map, _) = build_support_map::<AccountId>(&winners, &staked); + + if iterations > 0 { + sp_phragmen::equalize( + &mut staked, + &mut support_map, + Zero::zero(), + iterations, + ); + } // apply custom tweaks. awesome for testing. tweak(&mut staked); diff --git a/substrate/frame/staking/src/offchain_election.rs b/substrate/frame/staking/src/offchain_election.rs index 572703f895ec70a3a25f8192f0e0406d58d43975..2568638319394e45f72bdc844c790bd86bf3fad8 100644 --- a/substrate/frame/staking/src/offchain_election.rs +++ b/substrate/frame/staking/src/offchain_election.rs @@ -168,13 +168,7 @@ pub fn prepare_submission<T: Trait>( <Module<T>>::slashable_balance_of_vote_weight, ); - // reduce - if do_reduce { - reduce(&mut staked); - } - let (mut support_map, _) = build_support_map::<T::AccountId>(&winners, &staked); - // equalize a random number of times. let iterations_executed = match T::MaxIterations::get() { 0 => { @@ -194,6 +188,10 @@ pub fn prepare_submission<T: Trait>( } }; + // reduce + if do_reduce { + reduce(&mut staked); + } // Convert back to ratio assignment. This takes less space. let low_accuracy_assignment = sp_phragmen::assignment_staked_to_ratio(staked); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index f3dddc87ed776ba40925f8691663b7f5c862bc7b..73fcb6c2afeb9fe182dafebe450715938c045b24 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -2917,7 +2917,7 @@ mod offchain_phragmen { assert_eq!(Staking::era_election_status(), ElectionStatus::Open(12)); assert!(Staking::snapshot_validators().is_some()); - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); assert_ok!(Staking::submit_election_solution( Origin::signed(10), winners, @@ -2960,7 +2960,7 @@ mod offchain_phragmen { run_to_block(14); assert_eq!(Staking::era_election_status(), ElectionStatus::Open(12)); - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); assert_ok!(Staking::submit_election_solution( Origin::signed(10), winners, @@ -3007,7 +3007,7 @@ mod offchain_phragmen { // create all the indices just to build the solution. Staking::create_stakers_snapshot(); - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); Staking::kill_stakers_snapshot(); assert_noop!( @@ -3036,7 +3036,7 @@ mod offchain_phragmen { run_to_block(12); // a good solution - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); assert_ok!(Staking::submit_election_solution( Origin::signed(10), winners, @@ -3083,7 +3083,7 @@ mod offchain_phragmen { )); // a better solution - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); assert_ok!(Staking::submit_election_solution( Origin::signed(10), winners, @@ -3189,7 +3189,7 @@ mod offchain_phragmen { ext.execute_with(|| { run_to_block(12); // put a good solution on-chain - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); assert_ok!(Staking::submit_election_solution( Origin::signed(10), winners, @@ -3235,7 +3235,7 @@ mod offchain_phragmen { run_to_block(12); ValidatorCount::put(3); - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); ValidatorCount::put(4); assert_eq!(winners.len(), 3); @@ -3266,7 +3266,7 @@ mod offchain_phragmen { run_to_block(12); ValidatorCount::put(3); - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); ValidatorCount::put(4); assert_eq!(winners.len(), 3); @@ -3296,7 +3296,7 @@ mod offchain_phragmen { build_offchain_phragmen_test_ext(); run_to_block(12); - let (compact, winners, score) = prepare_submission_with(true, |_| {}); + let (compact, winners, score) = prepare_submission_with(true, 2, |_| {}); assert_eq!(winners.len(), 4); @@ -3325,7 +3325,7 @@ mod offchain_phragmen { assert_eq!(Staking::snapshot_nominators().unwrap().len(), 5 + 4); assert_eq!(Staking::snapshot_validators().unwrap().len(), 4); - let (mut compact, winners, score) = prepare_submission_with(true, |_| {}); + let (mut compact, winners, score) = prepare_submission_with(true, 2, |_| {}); // index 9 doesn't exist. compact.votes1.push((9, 2)); @@ -3358,7 +3358,7 @@ mod offchain_phragmen { assert_eq!(Staking::snapshot_nominators().unwrap().len(), 5 + 4); assert_eq!(Staking::snapshot_validators().unwrap().len(), 4); - let (mut compact, winners, score) = prepare_submission_with(true, |_| {}); + let (mut compact, winners, score) = prepare_submission_with(true, 2, |_| {}); // index 4 doesn't exist. compact.votes1.push((3, 4)); @@ -3391,7 +3391,7 @@ mod offchain_phragmen { assert_eq!(Staking::snapshot_nominators().unwrap().len(), 5 + 4); assert_eq!(Staking::snapshot_validators().unwrap().len(), 4); - let (compact, _, score) = prepare_submission_with(true, |_| {}); + let (compact, _, score) = prepare_submission_with(true, 2, |_| {}); // index 4 doesn't exist. let winners = vec![0, 1, 2, 4]; @@ -3424,7 +3424,7 @@ mod offchain_phragmen { assert_eq!(Staking::snapshot_nominators().unwrap().len(), 5 + 4); assert_eq!(Staking::snapshot_validators().unwrap().len(), 4); - let (compact, winners, score) = prepare_submission_with(true, |a| { + let (compact, winners, score) = prepare_submission_with(true, 2, |a| { a.iter_mut() .find(|x| x.who == 5) // all 3 cannot be among the winners. Although, all of them are validator @@ -3457,7 +3457,7 @@ mod offchain_phragmen { build_offchain_phragmen_test_ext(); run_to_block(12); - let (compact, winners, score) = prepare_submission_with(true, |a| { + let (compact, winners, score) = prepare_submission_with(true, 2, |a| { // mutate a self vote to target someone else. That someone else is still among the // winners a.iter_mut().find(|x| x.who == 11).map(|x| { @@ -3493,7 +3493,7 @@ mod offchain_phragmen { build_offchain_phragmen_test_ext(); run_to_block(12); - let (compact, winners, score) = prepare_submission_with(true, |a| { + let (compact, winners, score) = prepare_submission_with(true, 2, |a| { // Remove the self vote. a.retain(|x| x.who != 11); // add is as a new double vote @@ -3531,7 +3531,7 @@ mod offchain_phragmen { // Note: we don't reduce here to be able to tweak votes3. votes3 will vanish if you // reduce. - let (mut compact, winners, score) = prepare_submission_with(false, |_| {}); + let (mut compact, winners, score) = prepare_submission_with(false, 0, |_| {}); if let Some(c) = compact.votes3.iter_mut().find(|x| x.0 == 0) { // by default it should have been (0, [(2, 33%), (1, 33%)], 0) @@ -3573,7 +3573,7 @@ mod offchain_phragmen { build_offchain_phragmen_test_ext(); run_to_block(12); - let (compact, winners, score) = prepare_submission_with(false, |a| { + let (compact, winners, score) = prepare_submission_with(false, 0, |a| { // 3 only voted for 20 and 40. We add a fake vote to 30. The stake sum is still // correctly 100. a.iter_mut() @@ -3635,7 +3635,7 @@ mod offchain_phragmen { run_to_block(32); // a solution that has been prepared after the slash. - let (compact, winners, score) = prepare_submission_with(false, |a| { + let (compact, winners, score) = prepare_submission_with(false, 0, |a| { // no one is allowed to vote for 10, except for itself. a.into_iter() .filter(|s| s.who != 11) @@ -3654,7 +3654,7 @@ mod offchain_phragmen { )); // a wrong solution. - let (compact, winners, score) = prepare_submission_with(false, |a| { + let (compact, winners, score) = prepare_submission_with(false, 0, |a| { // add back the vote that has been filtered out. a.push(StakedAssignment { who: 1, @@ -3688,7 +3688,7 @@ mod offchain_phragmen { build_offchain_phragmen_test_ext(); run_to_block(12); - let (compact, winners, mut score) = prepare_submission_with(true, |_| {}); + let (compact, winners, mut score) = prepare_submission_with(true, 2, |_| {}); score[0] += 1; assert_noop!( diff --git a/substrate/primitives/phragmen/src/lib.rs b/substrate/primitives/phragmen/src/lib.rs index cf972eb1ed7babd138693f6ccd42fc2c61c24ab1..9aaa96150ff7471a56cfaed0137c1ad1df2483d0 100644 --- a/substrate/primitives/phragmen/src/lib.rs +++ b/substrate/primitives/phragmen/src/lib.rs @@ -599,11 +599,11 @@ pub fn evaluate_support<AccountId>( ) -> PhragmenScore { let mut min_support = ExtendedBalance::max_value(); let mut sum: ExtendedBalance = Zero::zero(); - // NOTE: this will probably saturate but using big num makes it even slower. We'll have to see. - // This must run on chain.. + // NOTE: The third element might saturate but fine for now since this will run on-chain and need + // to be fast. let mut sum_squared: ExtendedBalance = Zero::zero(); for (_, support) in support.iter() { - sum += support.total; + sum = sum.saturating_add(support.total); let squared = support.total.saturating_mul(support.total); sum_squared = sum_squared.saturating_add(squared); if support.total < min_support {