diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index ccd2eba78f..ba219a1890 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -83,7 +83,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 239, + spec_version: 240, impl_version: 0, apis: RUNTIME_API_VERSIONS, }; diff --git a/substrate/frame/elections-phragmen/src/lib.rs b/substrate/frame/elections-phragmen/src/lib.rs index d1a3c6af5f..d74fb4bdcd 100644 --- a/substrate/frame/elections-phragmen/src/lib.rs +++ b/substrate/frame/elections-phragmen/src/lib.rs @@ -158,13 +158,11 @@ decl_storage! { /// The total number of vote rounds that have happened, excluding the upcoming one. pub ElectionRounds get(fn election_rounds): u32 = Zero::zero(); - /// Votes of a particular voter, with the round index of the votes. - pub VotesOf get(fn votes_of): map hasher(twox_64_concat) T::AccountId => Vec; - /// Locked stake of a voter. - pub StakeOf get(fn stake_of): map hasher(twox_64_concat) T::AccountId => BalanceOf; + /// Votes and locked stake of a particular voter. + pub Voting: map hasher(twox_64_concat) T::AccountId => (BalanceOf, Vec); - /// The present candidate list. Sorted based on account-id. A current member or a runner can - /// never enter this vector and is always implicitly assumed to be a candidate. + /// The present candidate list. Sorted based on account-id. A current member or runner-up + /// can never enter this vector and is always implicitly assumed to be a candidate. pub Candidates get(fn candidates): Vec; } } @@ -203,12 +201,32 @@ decl_error! { } } +mod migration { + use super::*; + use frame_support::{migration::{StorageKeyIterator, take_storage_item}, Twox64Concat}; + pub fn migrate() { + for (who, votes) in StorageKeyIterator + ::, Twox64Concat> + ::new(b"PhragmenElection", b"VotesOf") + .drain() + { + if let Some(stake) = take_storage_item::<_, BalanceOf, Twox64Concat>(b"PhragmenElection", b"StakeOf", &who) { + Voting::::insert(who, (stake, votes)); + } + } + } +} + decl_module! { pub struct Module for enum Call where origin: T::Origin { type Error = Error; fn deposit_event() = default; + fn on_runtime_upgrade() { + migration::migrate::(); + } + const CandidacyBond: BalanceOf = T::CandidacyBond::get(); const VotingBond: BalanceOf = T::VotingBond::get(); const DesiredMembers: u32 = T::DesiredMembers::get(); @@ -264,8 +282,8 @@ decl_module! { locked_balance, WithdrawReasons::except(WithdrawReason::TransactionPayment), ); - >::insert(&who, locked_balance); - >::insert(&who, votes); + + Voting::::insert(&who, (locked_balance, votes)); } /// Remove `origin` as a voter. This removes the lock and returns the bond. @@ -522,7 +540,7 @@ impl Module { /// /// State: O(1). fn is_voter(who: &T::AccountId) -> bool { - >::contains_key(who) + Voting::::contains_key(who) } /// Check if `who` is currently an active member. @@ -585,8 +603,7 @@ impl Module { /// lock. Optionally, it would also return the reserved voting bond if indicated by `unreserve`. fn do_remove_voter(who: &T::AccountId, unreserve: bool) { // remove storage and lock. - >::remove(who); - >::remove(who); + Voting::::remove(who); T::Currency::remove_lock(MODULE_ID, who); if unreserve { @@ -596,7 +613,12 @@ impl Module { /// The locked stake of a voter. fn locked_stake_of(who: &T::AccountId) -> BalanceOf { - Self::stake_of(who) + Voting::::get(who).0 + } + + /// The locked stake of a voter. + fn votes_of(who: &T::AccountId) -> Vec { + Voting::::get(who).1 } /// Check there's nothing to do this block. @@ -627,7 +649,7 @@ impl Module { let num_to_elect = desired_runners_up + desired_seats; let mut candidates = Self::candidates(); - // candidates who explicitly called `submit_candidacy`. Only these folks are at the risk of + // candidates who explicitly called `submit_candidacy`. Only these folks are at risk of // losing their bond. let exposed_candidates = candidates.clone(); // current members are always a candidate for the next round as well. @@ -636,15 +658,14 @@ impl Module { // previous runners_up are also always candidates for the next round. candidates.append(&mut Self::runners_up_ids()); - let voters_and_votes = VotesOf::::iter() - .map(|(v, i)| (v, i)) - .collect::)>>(); - let maybe_phragmen_result = sp_phragmen::elect::<_, _, _, T::CurrencyToVote, Perbill>( + let voters_and_votes = Voting::::iter() + .map(|(voter, (stake, targets))| { (voter, stake, targets) }) + .collect::>(); + let maybe_phragmen_result = sp_phragmen::elect::<_, _, T::CurrencyToVote, Perbill>( num_to_elect, 0, candidates, - voters_and_votes, - Self::locked_stake_of, + voters_and_votes.clone(), ); if let Some(phragmen_result) = maybe_phragmen_result { @@ -689,12 +710,24 @@ impl Module { // split new set into winners and runners up. let split_point = desired_seats.min(new_set_with_stake.len()); let mut new_members = (&new_set_with_stake[..split_point]).to_vec(); - let most_popular = new_members.first().map(|x| x.0.clone()); // save the runners up as-is. They are sorted based on desirability. // sort and save the members. new_members.sort_by(|i, j| i.0.cmp(&j.0)); + let mut prime_votes: Vec<_> = new_members.iter().map(|c| (&c.0, BalanceOf::::zero())).collect(); + for (_, stake, targets) in voters_and_votes.into_iter() { + for (votes, who) in targets.iter() + .enumerate() + .map(|(votes, who)| ((MAXIMUM_VOTE - votes) as u32, who)) + { + if let Ok(i) = prime_votes.binary_search_by_key(&who, |k| k.0) { + prime_votes[i].1 += stake * votes.into(); + } + } + } + let prime = prime_votes.into_iter().max_by_key(|x| x.1).map(|x| x.0.clone()); + // new_members_ids is sorted by account id. let new_members_ids = new_members .iter() @@ -722,7 +755,7 @@ impl Module { &outgoing.clone(), &new_members_ids, ); - T::ChangeMembers::set_prime(most_popular); + T::ChangeMembers::set_prime(prime); // outgoing candidates lose their bond. let mut to_burn_bond = outgoing.to_vec(); @@ -1012,7 +1045,7 @@ mod tests { } fn all_voters() -> Vec { - >::iter().map(|(v, _)| v).collect::>() + Voting::::iter().map(|(v, _)| v).collect::>() } fn balances(who: &u64) -> (u64, u64) { @@ -1231,13 +1264,13 @@ mod tests { assert_eq!(balances(&2), (18, 2)); assert_eq!(has_lock(&2), 20); - assert_eq!(Elections::stake_of(2), 20); + assert_eq!(Elections::locked_stake_of(&2), 20); // can update; different stake; different lock and reserve. assert_ok!(Elections::vote(Origin::signed(2), vec![5, 4], 15)); assert_eq!(balances(&2), (18, 2)); assert_eq!(has_lock(&2), 15); - assert_eq!(Elections::stake_of(2), 15); + assert_eq!(Elections::locked_stake_of(&2), 15); }); } @@ -1293,6 +1326,31 @@ mod tests { }); } + #[test] + fn prime_votes_for_exiting_members_are_removed() { + ExtBuilder::default().build().execute_with(|| { + assert_ok!(Elections::submit_candidacy(Origin::signed(3))); + assert_ok!(Elections::submit_candidacy(Origin::signed(4))); + assert_ok!(Elections::submit_candidacy(Origin::signed(5))); + + assert_ok!(Elections::vote(Origin::signed(1), vec![4, 3], 10)); + assert_ok!(Elections::vote(Origin::signed(2), vec![4], 20)); + assert_ok!(Elections::vote(Origin::signed(3), vec![3], 30)); + assert_ok!(Elections::vote(Origin::signed(4), vec![4], 40)); + assert_ok!(Elections::vote(Origin::signed(5), vec![5], 50)); + + assert_ok!(Elections::renounce_candidacy(Origin::signed(4))); + + System::set_block_number(5); + assert_ok!(Elections::end_block(System::block_number())); + + assert_eq!(Elections::members_ids(), vec![3, 5]); + assert_eq!(Elections::candidates(), vec![]); + + assert_eq!(PRIME.with(|p| *p.borrow()), Some(5)); + }); + } + #[test] fn cannot_vote_for_more_than_candidates() { ExtBuilder::default().build().execute_with(|| { @@ -1327,7 +1385,7 @@ mod tests { assert_ok!(Elections::vote(Origin::signed(2), vec![4, 5], 30)); // you can lie but won't get away with it. - assert_eq!(Elections::stake_of(2), 20); + assert_eq!(Elections::locked_stake_of(&2), 20); assert_eq!(has_lock(&2), 20); }); } @@ -1341,16 +1399,16 @@ mod tests { assert_ok!(Elections::vote(Origin::signed(3), vec![5], 30)); assert_eq_uvec!(all_voters(), vec![2, 3]); - assert_eq!(Elections::stake_of(2), 20); - assert_eq!(Elections::stake_of(3), 30); - assert_eq!(Elections::votes_of(2), vec![5]); - assert_eq!(Elections::votes_of(3), vec![5]); + assert_eq!(Elections::locked_stake_of(&2), 20); + assert_eq!(Elections::locked_stake_of(&3), 30); + assert_eq!(Elections::votes_of(&2), vec![5]); + assert_eq!(Elections::votes_of(&3), vec![5]); assert_ok!(Elections::remove_voter(Origin::signed(2))); assert_eq_uvec!(all_voters(), vec![3]); - assert_eq!(Elections::votes_of(2), vec![]); - assert_eq!(Elections::stake_of(2), 0); + assert_eq!(Elections::votes_of(&2), vec![]); + assert_eq!(Elections::locked_stake_of(&2), 0); assert_eq!(balances(&2), (20, 0)); assert_eq!(Balances::locks(&2).len(), 0); @@ -1521,9 +1579,9 @@ mod tests { assert_eq_uvec!(all_voters(), vec![2, 3, 4]); - assert_eq!(Elections::votes_of(2), vec![5]); - assert_eq!(Elections::votes_of(3), vec![3]); - assert_eq!(Elections::votes_of(4), vec![4]); + assert_eq!(Elections::votes_of(&2), vec![5]); + assert_eq!(Elections::votes_of(&3), vec![3]); + assert_eq!(Elections::votes_of(&4), vec![4]); assert_eq!(Elections::candidates(), vec![3, 4, 5]); assert_eq!(>::decode_len().unwrap(), 3); diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 4851768074..53ef7b41e4 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -1810,11 +1810,11 @@ impl Module { /// /// Assumes storage is coherent with the declaration. fn select_validators(current_era: EraIndex) -> Option> { - let mut all_nominators: Vec<(T::AccountId, Vec)> = Vec::new(); + let mut all_nominators: Vec<(T::AccountId, BalanceOf, Vec)> = Vec::new(); let mut all_validators_and_prefs = BTreeMap::new(); let mut all_validators = Vec::new(); for (validator, preference) in >::iter() { - let self_vote = (validator.clone(), vec![validator.clone()]); + let self_vote = (validator.clone(), Self::slashable_balance_of(&validator), vec![validator.clone()]); all_nominators.push(self_vote); all_validators_and_prefs.insert(validator.clone(), preference); all_validators.push(validator); @@ -1834,14 +1834,16 @@ impl Module { (nominator, targets) }); - all_nominators.extend(nominator_votes); + all_nominators.extend(nominator_votes.map(|(n, ns)| { + let s = Self::slashable_balance_of(&n); + (n, s, ns) + })); - let maybe_phragmen_result = sp_phragmen::elect::<_, _, _, T::CurrencyToVote, Perbill>( + let maybe_phragmen_result = sp_phragmen::elect::<_, _, T::CurrencyToVote, Perbill>( Self::validator_count() as usize, Self::minimum_validator_count().max(1) as usize, all_validators, all_nominators, - Self::slashable_balance_of, ); if let Some(phragmen_result) = maybe_phragmen_result { diff --git a/substrate/frame/support/src/storage/migration.rs b/substrate/frame/support/src/storage/migration.rs index 67df4d04c0..8e6beefa88 100644 --- a/substrate/frame/support/src/storage/migration.rs +++ b/substrate/frame/support/src/storage/migration.rs @@ -185,3 +185,12 @@ pub fn remove_storage_prefix(module: &[u8], item: &[u8], hash: &[u8]) { key[32..].copy_from_slice(hash); frame_support::storage::unhashed::kill_prefix(&key) } + +/// Get a particular value in storage by the `module`, the map's `item` name and the key `hash`. +pub fn take_storage_item( + module: &[u8], + item: &[u8], + key: K, +) -> Option { + take_storage_value(module, item, key.using_encoded(H::hash).as_ref()) +} diff --git a/substrate/primitives/phragmen/src/lib.rs b/substrate/primitives/phragmen/src/lib.rs index 23acec19da..aba714faeb 100644 --- a/substrate/primitives/phragmen/src/lib.rs +++ b/substrate/primitives/phragmen/src/lib.rs @@ -149,16 +149,14 @@ pub type SupportMap = BTreeMap>; /// responsibility of the caller to make sure only those candidates who have a sensible economic /// value are passed in. From the perspective of this function, a candidate can easily be among the /// winner with no backing stake. -pub fn elect( +pub fn elect( candidate_count: usize, minimum_candidate_count: usize, initial_candidates: Vec, - initial_voters: Vec<(AccountId, Vec)>, - stake_of: FS, + initial_voters: Vec<(AccountId, Balance, Vec)>, ) -> Option> where AccountId: Default + Ord + Member, Balance: Default + Copy + AtLeast32Bit, - for<'r> FS: Fn(&'r AccountId) -> Balance, C: Convert + Convert, R: PerThing, { @@ -191,8 +189,7 @@ pub fn elect( // collect voters. use `c_idx_cache` for fast access and aggregate `approval_stake` of // candidates. - voters.extend(initial_voters.into_iter().map(|(who, votes)| { - let voter_stake = stake_of(&who); + voters.extend(initial_voters.into_iter().map(|(who, voter_stake, votes)| { let mut edges: Vec> = Vec::with_capacity(votes.len()); for v in votes { if let Some(idx) = c_idx_cache.get(&v) { diff --git a/substrate/primitives/phragmen/src/mock.rs b/substrate/primitives/phragmen/src/mock.rs index b3110a5dba..66ef64a6c2 100644 --- a/substrate/primitives/phragmen/src/mock.rs +++ b/substrate/primitives/phragmen/src/mock.rs @@ -335,12 +335,11 @@ pub(crate) fn run_and_compare( min_to_elect: usize, ) { // run fixed point code. - let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote, Perbill>( + let PhragmenResult { winners, assignments } = elect::<_, _, TestCurrencyToVote, Perbill>( to_elect, min_to_elect, candidates.clone(), - voters.clone(), - &stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); // run float poc code. diff --git a/substrate/primitives/phragmen/src/tests.rs b/substrate/primitives/phragmen/src/tests.rs index 9027cc335f..8bcf007c7b 100644 --- a/substrate/primitives/phragmen/src/tests.rs +++ b/substrate/primitives/phragmen/src/tests.rs @@ -80,12 +80,12 @@ fn phragmen_poc_works() { (30, vec![2, 3]), ]; - let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote, Output>( + let stake_of = create_stake_of(&[(10, 10), (20, 20), (30, 30)]); + let PhragmenResult { winners, assignments } = elect::<_, _, TestCurrencyToVote, Output>( 2, 2, candidates, - voters, - create_stake_of(&[(10, 10), (20, 20), (30, 30)]), + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); assert_eq_uvec!(winners, vec![(2, 40), (3, 50)]); @@ -149,12 +149,11 @@ fn phragmen_accuracy_on_large_scale_only_validators() { (5, (u64::max_value() - 2).into()), ]); - let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote, Output>( + let PhragmenResult { winners, assignments } = elect::<_, _, TestCurrencyToVote, Output>( 2, 2, candidates.clone(), - auto_generate_self_voters(&candidates), - stake_of, + auto_generate_self_voters(&candidates).iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); assert_eq_uvec!(winners, vec![(1, 18446744073709551614u128), (5, 18446744073709551613u128)]); @@ -180,12 +179,11 @@ fn phragmen_accuracy_on_large_scale_validators_and_nominators() { (14, u64::max_value().into()), ]); - let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote, Output>( + let PhragmenResult { winners, assignments } = elect::<_, _, TestCurrencyToVote, Output>( 2, 2, candidates, - voters, - stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); assert_eq_uvec!(winners, vec![(2, 36893488147419103226u128), (1, 36893488147419103219u128)]); @@ -212,12 +210,11 @@ fn phragmen_accuracy_on_small_scale_self_vote() { (30, 1), ]); - let PhragmenResult { winners, assignments: _ } = elect::<_, _, _, TestCurrencyToVote, Output>( + let PhragmenResult { winners, assignments: _ } = elect::<_, _, TestCurrencyToVote, Output>( 3, 3, candidates, - voters, - stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); assert_eq_uvec!(winners, vec![(20, 2), (10, 1), (30, 1)]); @@ -243,12 +240,11 @@ fn phragmen_accuracy_on_small_scale_no_self_vote() { (3, 1), ]); - let PhragmenResult { winners, assignments: _ } = elect::<_, _, _, TestCurrencyToVote, Output>( + let PhragmenResult { winners, assignments: _ } = elect::<_, _, TestCurrencyToVote, Output>( 3, 3, candidates, - voters, - stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); assert_eq_uvec!(winners, vec![(20, 2), (10, 1), (30, 1)]); @@ -277,12 +273,11 @@ fn phragmen_large_scale_test() { (50, 990000000000000000), ]); - let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote, Output>( + let PhragmenResult { winners, assignments } = elect::<_, _, TestCurrencyToVote, Output>( 2, 2, candidates, - voters, - stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); assert_eq_uvec!(winners, vec![(24, 1490000000000200000u128), (22, 1490000000000100000u128)]); @@ -304,12 +299,11 @@ fn phragmen_large_scale_test_2() { (50, nom_budget.into()), ]); - let PhragmenResult { winners, assignments } = elect::<_, _, _, TestCurrencyToVote, Output>( + let PhragmenResult { winners, assignments } = elect::<_, _, TestCurrencyToVote, Output>( 2, 2, candidates, - voters, - stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); assert_eq_uvec!(winners, vec![(2, 1000000000004000000u128), (4, 1000000000004000000u128)]); @@ -369,12 +363,11 @@ fn elect_has_no_entry_barrier() { (2, 10), ]); - let PhragmenResult { winners, assignments: _ } = elect::<_, _, _, TestCurrencyToVote, Output>( + let PhragmenResult { winners, assignments: _ } = elect::<_, _, TestCurrencyToVote, Output>( 3, 3, candidates, - voters, - stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); // 30 is elected with stake 0. The caller is responsible for stripping this. @@ -397,12 +390,11 @@ fn minimum_to_elect_is_respected() { (2, 10), ]); - let maybe_result = elect::<_, _, _, TestCurrencyToVote, Output>( + let maybe_result = elect::<_, _, TestCurrencyToVote, Output>( 10, 10, candidates, - voters, - stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ); assert!(maybe_result.is_none()); @@ -424,12 +416,11 @@ fn self_votes_should_be_kept() { (1, 8), ]); - let result = elect::<_, _, _, TestCurrencyToVote, Output>( + let result = elect::<_, _, TestCurrencyToVote, Output>( 2, 2, candidates, - voters, - &stake_of, + voters.iter().map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone())).collect::>(), ).unwrap(); assert_eq!(result.winners, vec![(20, 28), (10, 18)]);