Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Abstracts elections-phragmen pallet to use NposSolver #12588

Merged
merged 50 commits into from
Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
b714023
Abstracts elections-phragmen pallet to use NposSolver
gpestana Oct 31, 2022
452292f
Update frame/elections-phragmen/src/lib.rs
gpestana Nov 8, 2022
bf8d1c2
Update frame/elections-phragmen/src/lib.rs
gpestana Nov 8, 2022
5fa443e
changes the name of the pallet; adds changelog
gpestana Nov 8, 2022
f71c095
update changelog
gpestana Nov 8, 2022
e37206c
Adds weight testing
gpestana Nov 9, 2022
f7e970d
Adds log macro_rules
gpestana Nov 9, 2022
7b86c86
renames elections-phragment dir to elections
gpestana Nov 9, 2022
7ab9113
Merge remote-tracking branch 'origin/master' into gpestana8250_nposso…
Nov 13, 2022
e21f6ae
weights rename
gpestana Nov 13, 2022
e9c3daf
fixes typo in cargo toml
gpestana Nov 14, 2022
5d947af
pre/post solve weight scafolding
gpestana Nov 15, 2022
ecd7b63
refactor do_post_election
gpestana Nov 15, 2022
090aa90
refactors into pre and post election solve for independent benchmarking
gpestana Nov 15, 2022
1e2ef46
deconstructs PreElectionResults struct
gpestana Nov 15, 2022
d4b8c4e
updates benchmarking pre and post election solve; mock weights
gpestana Nov 16, 2022
c8a3508
Update frame/elections/src/lib.rs
gpestana Nov 17, 2022
8021572
Update frame/elections/src/lib.rs
gpestana Nov 17, 2022
c3db2cb
addresses PR comments
gpestana Nov 17, 2022
482dde3
adds pre_solve and post_sove weights
gpestana Nov 17, 2022
9749d12
Adds comments on election pallet id param name change
gpestana Nov 20, 2022
cbc8386
resolves conflict
gpestana Nov 20, 2022
4d51390
Merge branch 'master' of https://github.com/paritytech/substrate into…
Nov 21, 2022
26f9848
".git/.scripts/bench-bot.sh" pallet dev pallet_elections
Nov 22, 2022
afce2d2
Finishes pre-post solve weights
gpestana Nov 22, 2022
4208375
Update frame/elections/src/lib.rs
gpestana Nov 29, 2022
85e5a70
Update frame/elections/src/lib.rs
gpestana Nov 29, 2022
443f73d
Addresses PR comments: no panic in on_init path; nits
gpestana Nov 29, 2022
8945d96
Merge branch 'master' into gpestana8250_npossolver
gpestana Dec 8, 2022
7636999
Merge branch 'master' into gpestana8250_npossolver
gpestana Dec 12, 2022
0dc67e0
Merge branch 'master' into gpestana8250_npossolver
gpestana Jan 12, 2023
321b654
Fixes node build
gpestana Jan 12, 2023
ac36d28
Merge remote-tracking branch 'origin/master' into gpestana8250_nposso…
Jan 12, 2023
ee19357
Merge branch 'master' into gpestana8250_npossolver
gpestana Feb 8, 2023
e94d7a5
Merge branch 'master' into gpestana8250_npossolver
gpestana Feb 8, 2023
69111d9
Merge branch 'master' into gpestana8250_npossolver
gpestana Feb 14, 2023
bfe6fd9
Merge branch 'master' into gpestana8250_npossolver
gpestana Feb 15, 2023
8a1ebac
Merge branch 'master' into gpestana8250_npossolver
gpestana Feb 17, 2023
5343a6c
Implements approval voting to use as a `NposSolver` (#13367)
gpestana Feb 17, 2023
59900fc
fmt
gpestana Feb 20, 2023
da3339e
remove unecessary file
gpestana Feb 20, 2023
fea82e8
comment clarification
gpestana Feb 20, 2023
c1f5225
Merge remote-tracking branch 'origin/master' into gpestana8250_nposso…
Feb 20, 2023
64df412
re-run weights
Feb 20, 2023
614fe8a
fix typo
Feb 20, 2023
04483c6
updates MaxVoters in tests for integrity_tests to pass
gpestana Feb 20, 2023
51258d2
Merge remote-tracking branch 'origin/master' into gpestana8250_nposso…
Feb 21, 2023
281f7b8
Merge branch 'master' into gpestana8250_npossolver
gpestana Feb 22, 2023
70ea87a
Refactors election provider support benchmarks outside its own crate …
gpestana Feb 22, 2023
49e36db
Merge branch 'master' into gpestana8250_npossolver
gpestana Feb 22, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions frame/elections-phragmen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys
sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" }
sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" }
sp-npos-elections = { version = "4.0.0-dev", default-features = false, path = "../../primitives/npos-elections" }
frame-election-provider-support = { version = "4.0.0-dev", default-features = false, path = "../election-provider-support" }
sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" }
sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" }

Expand Down
123 changes: 73 additions & 50 deletions frame/elections-phragmen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! # Phragmén Election Module.
//! # Generic Election Module.
//!
//! An election module based on sequential phragmen.
//! An election module based on a generic election algorithm.
//!
//! ### Term and Round
//!
Expand Down Expand Up @@ -99,6 +99,7 @@
#![cfg_attr(not(feature = "std"), no_std)]

use codec::{Decode, Encode};
use frame_election_provider_support::NposSolver;
use frame_support::{
traits::{
defensive_prelude::*, ChangeMembers, Contains, ContainsLengthBound, Currency,
Expand All @@ -111,7 +112,7 @@ use scale_info::TypeInfo;
use sp_npos_elections::{ElectionResult, ExtendedBalance};
use sp_runtime::{
traits::{Saturating, StaticLookup, Zero},
DispatchError, Perbill, RuntimeDebug,
DispatchError, RuntimeDebug,
};
use sp_std::{cmp::Ordering, prelude::*};

Expand Down Expand Up @@ -197,7 +198,7 @@ pub mod pallet {
pub trait Config: frame_system::Config {
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

/// Identifier for the elections-phragmen pallet's lock
/// Identifier for the elections pallet's lock
gpestana marked this conversation as resolved.
Show resolved Hide resolved
#[pallet::constant]
type PalletId: Get<LockIdentifier>;

Expand Down Expand Up @@ -250,21 +251,27 @@ pub mod pallet {
#[pallet::constant]
type TermDuration: Get<Self::BlockNumber>;

/// The maximum number of candidates in a phragmen election.
/// The maximum number of candidates in an election.
///
/// Warning: The election happens onchain, and this value will determine
/// the size of the election. When this limit is reached no more
/// candidates are accepted in the election.
#[pallet::constant]
type MaxCandidates: Get<u32>;

/// The maximum number of voters to allow in a phragmen election.
/// The maximum number of voters to allow in an election.
///
/// Warning: This impacts the size of the election which is run onchain.
/// When the limit is reached the new voters are ignored.
#[pallet::constant]
type MaxVoters: Get<u32>;

/// Something that will calculate the result of elections.
type ElectionSolver: NposSolver<AccountId = Self::AccountId>;

/// Weight information for the `ElectionSolver`.
gpestana marked this conversation as resolved.
Show resolved Hide resolved
type SolverWeightInfo: frame_election_provider_support::WeightInfo;

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;
}
Expand All @@ -277,7 +284,7 @@ pub mod pallet {
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()
Self::do_election()
} else {
Weight::zero()
}
Expand Down Expand Up @@ -486,8 +493,8 @@ pub mod pallet {
/// the outgoing member is slashed.
///
/// If a runner-up is available, then the best runner-up will be removed and replaces the
/// outgoing member. Otherwise, if `rerun_election` is `true`, a new phragmen election is
/// started, else, nothing happens.
/// outgoing member. Otherwise, if `rerun_election` is `true`, a new election is started,
/// else, nothing happens.
///
/// If `slash_bond` is set to true, the bond of the member being removed is slashed. Else,
/// it is returned.
Expand All @@ -498,7 +505,7 @@ pub mod pallet {
///
/// # <weight>
/// If we have a replacement, we use a small weight. Else, since this is a root call and
/// will go into phragmen, we assume full block for now.
/// will go into the `NposSolver` election, we assume full block for now.
gpestana marked this conversation as resolved.
Show resolved Hide resolved
/// # </weight>
#[pallet::weight(if *rerun_election {
T::WeightInfo::remove_member_without_replacement()
Expand All @@ -518,7 +525,7 @@ pub mod pallet {
Self::deposit_event(Event::MemberKicked { member: who });

if rerun_election {
Self::do_phragmen();
Self::do_election();
}

// no refund needed.
Expand Down Expand Up @@ -695,7 +702,7 @@ pub mod pallet {
Members::<T>::mutate(|members| {
match members.binary_search_by(|m| m.who.cmp(member)) {
Ok(_) => {
panic!("Duplicate member in elections-phragmen genesis: {}", member)
panic!("Duplicate member in elections genesis: {}", member)
},
Err(pos) => members.insert(
pos,
Expand Down Expand Up @@ -784,7 +791,7 @@ impl<T: Config> Pallet<T> {
// 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.",
);
}
Expand Down Expand Up @@ -890,11 +897,12 @@ impl<T: Config> Pallet<T> {
debug_assert!(_remainder.is_zero());
}

/// Run the phragmen election with all required side processes and state updates, if election
/// succeeds. Else, it will emit an `ElectionError` event.
/// Run an election with all required side processes and state updates, if election
/// succeeds. Else, it will emit an `ElectionError` event. The election algorithm is defined
/// by the implementor of `Self::NposSolver`.
gpestana marked this conversation as resolved.
Show resolved Hide resolved
///
/// 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;
Expand All @@ -908,7 +916,7 @@ impl<T: Config> Pallet<T> {
return T::DbWeight::get().reads(3)
}

// All of the new winners that come out of phragmen will thus have a deposit recorded.
// All of the new winners that come out of the election will thus have a deposit recorded.
let candidate_ids =
candidates_and_deposit.iter().map(|(x, _)| x).cloned().collect::<Vec<_>>();

Expand All @@ -933,15 +941,15 @@ impl<T: Config> Pallet<T> {
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)
},
}

// used for phragmen.
// used for elections.
let voters_and_votes = voters_and_stakes
.iter()
.cloned()
Expand All @@ -951,12 +959,14 @@ impl<T: Config> Pallet<T> {
})
.collect::<Vec<_>>();

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::<T::AccountId, Perbill> { winners, assignments: _ }| {
let num_candidates = candidates_and_deposit.len() as u32;
let num_voters = voters_and_votes.len() as u32;
let _ = T::ElectionSolver::solve(num_to_elect, candidate_ids, voters_and_votes)
.map(
|ElectionResult::<T::AccountId, <T::ElectionSolver as NposSolver>::Accuracy> {
winners,
assignments: _,
}| {
// this is already sorted by id.
let old_members_ids_sorted = <Members<T>>::take()
.into_iter()
Expand Down Expand Up @@ -1059,7 +1069,7 @@ impl<T: Config> Pallet<T> {
// write final values to storage.
let deposit_of_candidate = |x: &T::AccountId| -> BalanceOf<T> {
// 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()
Expand Down Expand Up @@ -1095,17 +1105,18 @@ impl<T: Config> Pallet<T> {

Self::deposit_event(Event::NewTerm { new_members: new_members_sorted_by_id });
<ElectionRounds<T>>::mutate(|v| *v += 1);
})
.map_err(|e| {
log::error!(
target: "runtime::elections-phragmen",
"Failed to run election [{:?}].",
e,
);
Self::deposit_event(Event::ElectionError);
});
},
)
.map_err(|e| {
log::error!(
gpestana marked this conversation as resolved.
Show resolved Hide resolved
target: "runtime::elections",
"Failed to run election [{:?}].",
e,
);
Self::deposit_event(Event::ElectionError);
});

T::WeightInfo::election_phragmen(weight_candidates, weight_voters, weight_edges)
T::ElectionSolver::weight::<T::SolverWeightInfo>(num_voters, num_candidates, num_edges)
}
}

Expand Down Expand Up @@ -1156,7 +1167,8 @@ impl<T: Config> ContainsLengthBound for Pallet<T> {
#[cfg(test)]
mod tests {
use super::*;
use crate as elections_phragmen;
use crate as elections;
use frame_election_provider_support::SequentialPhragmen;
use frame_support::{
assert_noop, assert_ok,
dispatch::DispatchResultWithPostInfo,
Expand All @@ -1168,7 +1180,7 @@ mod tests {
use sp_runtime::{
testing::Header,
traits::{BlakeTwo256, IdentityLookup},
BuildStorage,
BuildStorage, Perbill,
};
use substrate_test_utils::assert_eq_uvec;

Expand Down Expand Up @@ -1274,13 +1286,13 @@ mod tests {
}

parameter_types! {
pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect";
pub const PhragmenMaxVoters: u32 = 1000;
pub const PhragmenMaxCandidates: u32 = 100;
pub const ElectionsPalletId: LockIdentifier = *b"elects__";
pub const MaxVoters: u32 = 1000;
pub const MaxCandidates: u32 = 100;
}

impl Config for Test {
type PalletId = ElectionsPhragmenPalletId;
type PalletId = ElectionsPalletId;
type RuntimeEvent = RuntimeEvent;
type Currency = Balances;
type CurrencyToVote = frame_support::traits::SaturatingCurrencyToVote;
Expand All @@ -1295,8 +1307,21 @@ mod tests {
type LoserCandidate = ();
type KickedMember = ();
type WeightInfo = ();
type MaxVoters = PhragmenMaxVoters;
type MaxCandidates = PhragmenMaxCandidates;
type MaxVoters = MaxVoters;
type MaxCandidates = MaxCandidates;
type ElectionSolver = SequentialPhragmen<Self::AccountId, Perbill>;
type SolverWeightInfo = NposWeightInfo;
}

pub struct NposWeightInfo;
impl frame_election_provider_support::WeightInfo for NposWeightInfo {
gpestana marked this conversation as resolved.
Show resolved Hide resolved
fn phragmen(_v: u32, _t: u32, _d: u32) -> Weight {
Weight::zero()
}

fn phragmms(_v: u32, _t: u32, _d: u32) -> Weight {
Weight::zero()
}
}

pub type Block = sp_runtime::generic::Block<Header, UncheckedExtrinsic>;
Expand All @@ -1311,7 +1336,7 @@ mod tests {
{
System: frame_system::{Pallet, Call, Event<T>},
Balances: pallet_balances::{Pallet, Call, Event<T>, Config<T>},
Elections: elections_phragmen::{Pallet, Call, Event<T>, Config<T>},
Elections: elections::{Pallet, Call, Event<T>, Config<T>},
}
);

Expand Down Expand Up @@ -1372,9 +1397,7 @@ mod tests {
(6, 60 * self.balance_factor),
],
},
elections: elections_phragmen::GenesisConfig::<Test> {
members: self.genesis_members,
},
elections: elections::GenesisConfig::<Test> { members: self.genesis_members },
}
.build_storage()
.unwrap()
Expand Down Expand Up @@ -1432,7 +1455,7 @@ mod tests {
.get(0)
.cloned()
.map(|lock| {
assert_eq!(lock.id, ElectionsPhragmenPalletId::get());
assert_eq!(lock.id, ElectionsPalletId::get());
lock.amount
})
.unwrap_or_default()
Expand Down Expand Up @@ -1623,7 +1646,7 @@ mod tests {
}

#[test]
#[should_panic = "Duplicate member in elections-phragmen genesis: 2"]
#[should_panic = "Duplicate member in elections genesis: 2"]
fn genesis_members_cannot_be_duplicate() {
ExtBuilder::default()
.desired_members(3)
Expand Down