diff --git a/Cargo.lock b/Cargo.lock index 6a5b562d0a791..eec5da76b874f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5424,6 +5424,7 @@ name = "pallet-elections-phragmen" version = "5.0.0-dev" dependencies = [ "frame-benchmarking", + "frame-election-provider-support", "frame-support", "frame-system", "log", diff --git a/frame/elections-phragmen/Cargo.toml b/frame/elections-phragmen/Cargo.toml index 2d71a6bed39df..995a8f252b303 100644 --- a/frame/elections-phragmen/Cargo.toml +++ b/frame/elections-phragmen/Cargo.toml @@ -20,6 +20,7 @@ log = { version = "0.4.14", default-features = false } scale-info = { version = "2.0.0", default-features = false, features = ["derive"] } frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } +frame-election-provider-support = { version = "4.0.0-dev", default-features = false, path = "../election-provider-support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 165a8fcab429b..4049e22b706a5 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -98,20 +98,27 @@ #![cfg_attr(not(feature = "std"), no_std)] +use core::marker::PhantomData; + use codec::{Decode, Encode}; +use frame_election_provider_support::{ + ElectionDataProvider, ElectionProvider, +}; use frame_support::{ traits::{ - defensive_prelude::*, ChangeMembers, Contains, ContainsLengthBound, Currency, + defensive_prelude::*, ChangeMembers, ConstU32, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency, SortedMembers, WithdrawReasons, }, weights::Weight, + BoundedVec, + dispatch::DispatchClass, }; use scale_info::TypeInfo; -use sp_npos_elections::{ElectionResult, ExtendedBalance}; +use sp_npos_elections::ExtendedBalance; use sp_runtime::{ - traits::{Saturating, StaticLookup, Zero}, - DispatchError, Perbill, RuntimeDebug, + traits::{Saturating, StaticLookup, Zero, UniqueSaturatedInto}, + DispatchError, RuntimeDebug, }; use sp_std::{cmp::Ordering, prelude::*}; @@ -123,7 +130,7 @@ pub use weights::WeightInfo; pub mod migrations; /// The maximum votes allowed per voter. -pub const MAXIMUM_VOTE: usize = 16; +pub const MAXIMUM_VOTE: u32 = 16; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; @@ -267,6 +274,14 @@ pub mod pallet { /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; + + /// Something that will calculate the election results based on the data provided by + /// `Self as DataProvider`. + type ElectionProvider: ElectionProvider< + AccountId = Self::AccountId, + BlockNumber = Self::BlockNumber, + DataProvider = Pallet, + >; } #[pallet::hooks] @@ -276,8 +291,8 @@ pub mod pallet { /// Checks if an election needs to happen or not. fn on_initialize(n: T::BlockNumber) -> Weight { let term_duration = T::TermDuration::get(); - if !term_duration.is_zero() && (n % term_duration).is_zero() { - Self::do_phragmen() + if !term_duration.is_zero() && Self::next_election_prediction(n) == n { + Self::do_election() } else { Weight::zero() } @@ -322,7 +337,7 @@ pub mod pallet { let who = ensure_signed(origin)?; // votes should not be empty and more than `MAXIMUM_VOTE` in any case. - ensure!(votes.len() <= MAXIMUM_VOTE, Error::::MaximumVotesExceeded); + ensure!(votes.len() as u32 <= MAXIMUM_VOTE, Error::::MaximumVotesExceeded); ensure!(!votes.is_empty(), Error::::NoVotes); let candidates_count = >::decode_len().unwrap_or(0); @@ -518,7 +533,7 @@ pub mod pallet { Self::deposit_event(Event::MemberKicked { member: who }); if rerun_election { - Self::do_phragmen(); + Self::do_election(); } // no refund needed. @@ -617,6 +632,8 @@ pub mod pallet { InvalidRenouncing, /// Prediction regarding replacement after member removal is wrong. InvalidReplacement, + // Errors from the data provider. + DataProvider, } /// The current elected members. @@ -784,7 +801,7 @@ impl Pallet { // overlap. This can never happen. If so, it seems like our intended replacement // is already a member, so not much more to do. log::error!( - target: "runtime::elections-phragmen", + target: "runtime::elections", "A member seems to also be a runner-up.", ); } @@ -890,14 +907,17 @@ impl Pallet { debug_assert!(_remainder.is_zero()); } - /// Run the phragmen election with all required side processes and state updates, if election + /// Run the election with all required side processes and state updates, if election /// succeeds. Else, it will emit an `ElectionError` event. /// /// Calls the appropriate [`ChangeMembers`] function variant internally. - fn do_phragmen() -> Weight { + fn do_election() -> Weight { let desired_seats = T::DesiredMembers::get() as usize; - let desired_runners_up = T::DesiredRunnersUp::get() as usize; - let num_to_elect = desired_runners_up + desired_seats; + let max_voters = ::MaxVoters::get() as usize; + + // helper closures to deal with balance/stake. + let total_issuance = T::Currency::total_issuance(); + let to_balance = |e: ExtendedBalance| T::CurrencyToVote::to_currency(e, total_issuance); let mut candidates_and_deposit = Self::candidates(); // add all the previous members and runners-up as candidates as well. @@ -905,22 +925,12 @@ impl Pallet { if candidates_and_deposit.len().is_zero() { Self::deposit_event(Event::EmptyTerm); - return T::DbWeight::get().reads(3) + return T::DbWeight::get().reads(3); } - // All of the new winners that come out of phragmen will thus have a deposit recorded. - let candidate_ids = - candidates_and_deposit.iter().map(|(x, _)| x).cloned().collect::>(); + // All of the new winners that come out of the election will thus have a deposit recorded. + let _ = candidates_and_deposit.iter().map(|(x, _)| x).cloned().collect::>(); - // helper closures to deal with balance/stake. - let total_issuance = T::Currency::total_issuance(); - let to_votes = |b: BalanceOf| T::CurrencyToVote::to_vote(b, total_issuance); - let to_balance = |e: ExtendedBalance| T::CurrencyToVote::to_currency(e, total_issuance); - - let mut num_edges: u32 = 0; - - let max_voters = ::MaxVoters::get() as usize; - // used for prime election. let mut voters_and_stakes = Vec::new(); match Voting::::iter().try_for_each(|(voter, Voter { stake, votes, .. })| { if voters_and_stakes.len() < max_voters { @@ -933,32 +943,24 @@ impl Pallet { Ok(_) => (), Err(_) => { log::error!( - target: "runtime::elections-phragmen", + target: "runtime::elections", "Failed to run election. Number of voters exceeded", ); Self::deposit_event(Event::ElectionError); - return T::DbWeight::get().reads(3 + max_voters as u64) + return T::DbWeight::get().reads(3 + max_voters as u64); }, } - // used for phragmen. - let voters_and_votes = voters_and_stakes - .iter() - .cloned() - .map(|(voter, stake, votes)| { - num_edges = num_edges.saturating_add(votes.len() as u32); - (voter, to_votes(stake), votes) - }) - .collect::>(); - let weight_candidates = candidates_and_deposit.len() as u32; - let weight_voters = voters_and_votes.len() as u32; - let weight_edges = num_edges; - let _ = - sp_npos_elections::seq_phragmen(num_to_elect, candidate_ids, voters_and_votes, None) - .map(|ElectionResult:: { winners, assignments: _ }| { - // this is already sorted by id. - let old_members_ids_sorted = >::take() + let weight_voters = voters_and_stakes.len() as u32; + let weight_edges = 0; // TODO(gpestana): generalize for elections + + let _ = T::ElectionProvider::elect().map(|mut winners| { + // sort winners by stake + winners.sort_by(|i, j| j.1.total.cmp(&i.1.total)); + + // this is already sorted by id. + let old_members_ids_sorted = >::take() .into_iter() .map(|m| m.who) .collect::>(); @@ -973,7 +975,7 @@ impl Pallet { let mut new_set_with_stake = winners .into_iter() .filter_map( - |(m, b)| if b.is_zero() { None } else { Some((m, to_balance(b))) }, + |(m, support)| if support.total.is_zero() { None } else { Some((m, to_balance(support.total))) }, ) .collect::)>>(); @@ -986,6 +988,7 @@ impl Pallet { let split_point = desired_seats.min(new_set_with_stake.len()); let mut new_members_sorted_by_id = new_set_with_stake.drain(..split_point).collect::>(); + new_members_sorted_by_id.sort_by(|i, j| i.0.cmp(&j.0)); // all the rest will be runners-up @@ -1009,7 +1012,7 @@ impl Pallet { for (_, stake, votes) in voters_and_stakes.into_iter() { for (vote_multiplier, who) in votes.iter().enumerate().map(|(vote_position, who)| { - ((MAXIMUM_VOTE - vote_position) as u32, who) + (MAXIMUM_VOTE - (vote_position as u32), who) }) { if let Ok(i) = prime_votes.binary_search_by_key(&who, |k| k.0) { prime_votes[i].1 = prime_votes[i] @@ -1018,6 +1021,7 @@ impl Pallet { } } } + // We then select the new member with the highest weighted stake. In the case of // a tie, the last person in the list with the tied score is selected. This is // the person with the "highest" account id based on the sort above. @@ -1044,8 +1048,8 @@ impl Pallet { // All candidates/members/runners-up who are no longer retaining a position as a // seat holder will lose their bond. candidates_and_deposit.iter().for_each(|(c, d)| { - if new_members_ids_sorted.binary_search(c).is_err() && - new_runners_up_ids_sorted.binary_search(c).is_err() + if new_members_ids_sorted.binary_search(c).is_err() + && new_runners_up_ids_sorted.binary_search(c).is_err() { let (imbalance, _) = T::Currency::slash_reserved(c, *d); T::LoserCandidate::on_unbalanced(imbalance); @@ -1059,13 +1063,14 @@ impl Pallet { // write final values to storage. let deposit_of_candidate = |x: &T::AccountId| -> BalanceOf { // defensive-only. This closure is used against the new members and new - // runners-up, both of which are phragmen winners and thus must have + // runners-up, both of which are election winners and thus must have // deposit. candidates_and_deposit .iter() .find_map(|(c, d)| if c == x { Some(*d) } else { None }) .defensive_unwrap_or_default() }; + // fetch deposits from the one recorded one. This will make sure that a // candidate who submitted candidacy before a change to candidacy deposit will // have the correct amount recorded. @@ -1092,21 +1097,33 @@ impl Pallet { // clean candidates. >::kill(); - + Self::deposit_event(Event::NewTerm { new_members: new_members_sorted_by_id }); >::mutate(|v| *v += 1); }) .map_err(|e| { log::error!( - target: "runtime::elections-phragmen", + target: "runtime::elections", "Failed to run election [{:?}].", e, ); Self::deposit_event(Event::ElectionError); - }); - T::WeightInfo::election_phragmen(weight_candidates, weight_voters, weight_edges) + }); + + T::WeightInfo::election(weight_candidates, weight_voters, weight_edges) } + + /// Register some amount of weight directly with the system pallet. + /// + /// This is always mandatory weight. + fn register_weight(weight: Weight) { + >::register_extra_weight_unchecked( + weight, + DispatchClass::Mandatory, + ); + } + } impl Contains for Pallet { @@ -1153,10 +1170,89 @@ impl ContainsLengthBound for Pallet { } } + +impl ElectionDataProvider for Pallet { + type AccountId = T::AccountId; + type BlockNumber = T::BlockNumber; + type MaxVotesPerVoter = ConstU32; + + fn electable_targets( + maybe_max_len: Option, + ) -> frame_election_provider_support::data_provider::Result> { + let mut candidates_and_deposit = Self::candidates(); + // add all the previous members and runners-up as candidates as well. + candidates_and_deposit.append(&mut Self::implicit_candidates_with_deposit()); + let mut targets: Vec<_> = candidates_and_deposit.into_iter().map(|(target, _)| target).collect(); + + Self::register_weight(T::WeightInfo::electable_candidates(targets.len() as u32)); + + if let Some(max_candidates) = maybe_max_len { + // TODO(gpestana):: or should return Err instead? + targets.truncate(max_candidates); + } + + Ok(targets) + } + + fn electing_voters( + maybe_max_len: Option, + ) -> frame_election_provider_support::data_provider::Result< + Vec>, + > { + let mut voters_and_stakes = Vec::new(); + + match Voting::::iter().try_for_each(|(voter, Voter { stake, votes, .. })| { + if let Some(max_voters) = maybe_max_len { + if voters_and_stakes.len() >= max_voters { + return Err(()); + } + } + let bounded_votes: BoundedVec<_, Self::MaxVotesPerVoter> = + BoundedVec::try_from(votes).map_err(|_| ())?; + + voters_and_stakes.push((voter, stake.unique_saturated_into(), bounded_votes)); + Ok(()) + }) { + Ok(_) => (), + Err(_) => { + log::error!( + target: "runtime::elections", + "Failed to run election. Number of voters exceeded", + ); + Self::deposit_event(Event::ElectionError); + Self::register_weight(T::WeightInfo::electing_voters(voters_and_stakes.len() as u32)); + }, + } + + Self::register_weight(T::WeightInfo::electing_voters(voters_and_stakes.len() as u32)); + + Ok(voters_and_stakes) + } + + fn desired_targets() -> frame_election_provider_support::data_provider::Result { + Self::register_weight(T::DbWeight::get().reads(2)); + + let desired_seats = T::DesiredMembers::get() as usize; + let desired_runners_up = T::DesiredRunnersUp::get() as usize; + Ok((desired_runners_up + desired_seats) as u32) + } + + fn next_election_prediction(now: Self::BlockNumber) -> Self::BlockNumber { + let term_duration = T::TermDuration::get(); + let blocks_to_next_election = now % term_duration; + + if blocks_to_next_election == Self::BlockNumber::zero() { + return now; + } + now + term_duration - (blocks_to_next_election) + } +} + #[cfg(test)] mod tests { use super::*; - use crate as elections_phragmen; + use crate::{self as elections_phragmen}; + use frame_election_provider_support::{SequentialPhragmen, onchain}; use frame_support::{ assert_noop, assert_ok, dispatch::DispatchResultWithPostInfo, @@ -1168,7 +1264,7 @@ mod tests { use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, - BuildStorage, + BuildStorage, Perbill, }; use substrate_test_utils::assert_eq_uvec; @@ -1227,6 +1323,8 @@ mod tests { pub static TermDuration: u64 = 5; pub static Members: Vec = vec![]; pub static Prime: Option = None; + pub static MaxVoters: u32 = 100; + pub static MaxTargets: u32 = 100; } pub struct TestChangeMembers; @@ -1277,6 +1375,7 @@ mod tests { pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect"; pub const PhragmenMaxVoters: u32 = 1000; pub const PhragmenMaxCandidates: u32 = 100; + pub const MaxVotesPerVoter: u32 = 16; } impl Config for Test { @@ -1297,11 +1396,26 @@ mod tests { type WeightInfo = (); type MaxVoters = PhragmenMaxVoters; type MaxCandidates = PhragmenMaxCandidates; + type ElectionProvider = onchain::BoundedExecution::; + } + + pub struct SeqPhragmenProvider; + impl onchain::Config for SeqPhragmenProvider{ + type System = Test; + type Solver = SequentialPhragmen; + type DataProvider = Elections; + type WeightInfo = (); + } + + impl onchain::BoundedConfig for SeqPhragmenProvider{ + type TargetsBound = MaxTargets; + type VotersBound = MaxVoters; } pub type Block = sp_runtime::generic::Block; pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; + pub type AccountId = u64; frame_support::construct_runtime!( pub enum Test where @@ -2230,8 +2344,10 @@ mod tests { assert_ok!(vote(RuntimeOrigin::signed(4), vec![4], 40)); System::set_block_number(5); + Elections::on_initialize(System::block_number()); + // failing here System::assert_last_event(RuntimeEvent::Elections(super::Event::NewTerm { new_members: vec![(4, 35), (5, 45)], })); @@ -3218,4 +3334,14 @@ mod tests { assert_ok!(Elections::clean_defunct_voters(RuntimeOrigin::root(), 4, 2)); }) } + + #[test] + fn election_data_provider_next_election_prediction() { + ExtBuilder::default().build_and_execute(|| { + assert_eq!(::next_election_prediction(1), 5); + assert_eq!(::next_election_prediction(5), 5); + assert_eq!(::next_election_prediction(7), 10); + assert_eq!(::next_election_prediction(11), 15); + }) + } } diff --git a/frame/elections-phragmen/src/migrations/v4.rs b/frame/elections-phragmen/src/migrations/v4.rs index 76ef630706c50..4a840a7ab9ce8 100644 --- a/frame/elections-phragmen/src/migrations/v4.rs +++ b/frame/elections-phragmen/src/migrations/v4.rs @@ -38,7 +38,7 @@ pub fn migrate>(new_pallet_name: N) -> Weight { target: "runtime::elections-phragmen", "New pallet name is equal to the old prefix. No migration needs to be done.", ); - return Weight::zero() + return Weight::zero(); } let storage_version = StorageVersion::get::>(); log::info!( diff --git a/frame/elections-phragmen/src/weights.rs b/frame/elections-phragmen/src/weights.rs index 52a5dedc723d4..0b6e9c561aa72 100644 --- a/frame/elections-phragmen/src/weights.rs +++ b/frame/elections-phragmen/src/weights.rs @@ -57,7 +57,9 @@ pub trait WeightInfo { fn remove_member_without_replacement() -> Weight; fn remove_member_with_replacement() -> Weight; fn clean_defunct_voters(v: u32, d: u32, ) -> Weight; - fn election_phragmen(c: u32, v: u32, e: u32, ) -> Weight; + fn election(c: u32, v: u32, e: u32, ) -> Weight; + fn electable_candidates(c: u32) -> Weight; + fn electing_voters(v: u32) -> Weight; } /// Weights for pallet_elections_phragmen using the Substrate node and recommended hardware. @@ -188,17 +190,28 @@ impl WeightInfo for SubstrateWeight { /// The range of component `c` is `[1, 1000]`. /// The range of component `v` is `[1, 10000]`. /// The range of component `e` is `[10000, 160000]`. - fn election_phragmen(c: u32, v: u32, e: u32, ) -> Weight { + fn election(c: u32, v: u32, e: u32, ) -> Weight { Weight::from_ref_time(0 as u64) // Standard Error: 773_000 .saturating_add(Weight::from_ref_time(81_534_000 as u64).saturating_mul(v as u64)) // Standard Error: 51_000 .saturating_add(Weight::from_ref_time(4_453_000 as u64).saturating_mul(e as u64)) .saturating_add(T::DbWeight::get().reads(280 as u64)) - .saturating_add(T::DbWeight::get().reads((1 as u64).saturating_mul(c as u64))) - .saturating_add(T::DbWeight::get().reads((1 as u64).saturating_mul(v as u64))) .saturating_add(T::DbWeight::get().writes((1 as u64).saturating_mul(c as u64))) } + + // Storage: Elections Candidates (r:1 w:0) + fn electable_candidates(c: u32) -> Weight { + Weight::from_ref_time(0 as u64) + .saturating_add(RocksDbWeight::get().reads(1 as u64).saturating_mul(c as u64)) + } + + // Storage: Elections Voting (r:1 w:0) + fn electing_voters(v: u32) -> Weight { + Weight::from_ref_time(0 as u64) + .saturating_add(RocksDbWeight::get().reads(1 as u64).saturating_mul(v as u64)) + } + } // For backwards compatibility and tests @@ -316,7 +329,7 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads((3 as u64).saturating_mul(v as u64))) .saturating_add(RocksDbWeight::get().writes((3 as u64).saturating_mul(v as u64))) } - // Storage: Elections Candidates (r:1 w:1) + // Storage: Elections Candidates (r:0 w:1) // Storage: Elections Members (r:1 w:1) // Storage: Elections RunnersUp (r:1 w:1) // Storage: Elections Voting (r:10001 w:0) @@ -328,15 +341,26 @@ impl WeightInfo for () { /// The range of component `c` is `[1, 1000]`. /// The range of component `v` is `[1, 10000]`. /// The range of component `e` is `[10000, 160000]`. - fn election_phragmen(c: u32, v: u32, e: u32, ) -> Weight { + fn election(c: u32, v: u32, e: u32, ) -> Weight { Weight::from_ref_time(0 as u64) // Standard Error: 773_000 .saturating_add(Weight::from_ref_time(81_534_000 as u64).saturating_mul(v as u64)) // Standard Error: 51_000 .saturating_add(Weight::from_ref_time(4_453_000 as u64).saturating_mul(e as u64)) .saturating_add(RocksDbWeight::get().reads(280 as u64)) - .saturating_add(RocksDbWeight::get().reads((1 as u64).saturating_mul(c as u64))) - .saturating_add(RocksDbWeight::get().reads((1 as u64).saturating_mul(v as u64))) .saturating_add(RocksDbWeight::get().writes((1 as u64).saturating_mul(c as u64))) } + + // Storage: Elections Candidates (r:1 w:0) + fn electable_candidates(c: u32) -> Weight { + Weight::from_ref_time(0 as u64) + .saturating_add(RocksDbWeight::get().reads(1 as u64).saturating_mul(c as u64)) + } + + // Storage: Elections Voting (r:1 w:0) + fn electing_voters(v: u32) -> Weight { + Weight::from_ref_time(0 as u64) + .saturating_add(RocksDbWeight::get().reads(1 as u64).saturating_mul(v as u64)) + } + }