From 69a8b372c68b7c0d9f0bb9a1fc4061ca0ee31274 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sun, 23 Oct 2022 16:30:29 +0200 Subject: [PATCH 01/34] Bound Phragmen --- frame/elections-phragmen/src/lib.rs | 97 ++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 29 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 165a8fcab429b..685e7e2c26114 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -100,12 +100,14 @@ use codec::{Decode, Encode}; use frame_support::{ + bounded_vec, traits::{ defensive_prelude::*, ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency, SortedMembers, WithdrawReasons, }, weights::Weight, + BoundedVec, }; use scale_info::TypeInfo; use sp_npos_elections::{ElectionResult, ExtendedBalance}; @@ -144,9 +146,9 @@ pub enum Renouncing { /// An active voter. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, TypeInfo)] -pub struct Voter { +pub struct Voter { /// The members being backed. - pub votes: Vec, + pub votes: BoundedVec, /// The amount of stake placed on this vote. pub stake: Balance, /// The amount of deposit reserved for this vote. @@ -155,9 +157,11 @@ pub struct Voter { pub deposit: Balance, } -impl Default for Voter { +impl> Default + for Voter +{ fn default() -> Self { - Self { votes: vec![], stake: Default::default(), deposit: Default::default() } + Self { votes: bounded_vec![], stake: Default::default(), deposit: Default::default() } } } @@ -265,6 +269,14 @@ pub mod pallet { #[pallet::constant] type MaxVoters: Get; + /// The maximum number of elected members. + #[pallet::constant] + type MaxMembers: Get; + + /// The maximum number of runners-up. + #[pallet::constant] + type MaxRunnersUp: Get; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -416,7 +428,10 @@ pub mod pallet { T::Currency::reserve(&who, T::CandidacyBond::get()) .map_err(|_| Error::::InsufficientCandidateFunds)?; - >::mutate(|c| c.insert(index, (who, T::CandidacyBond::get()))); + >::mutate(|c| { + c.try_insert(index, (who, T::CandidacyBond::get())) + .expect("Too many candidates!") + }); Ok(()) } @@ -624,8 +639,11 @@ pub mod pallet { /// Invariant: Always sorted based on account id. #[pallet::storage] #[pallet::getter(fn members)] - pub type Members = - StorageValue<_, Vec>>, ValueQuery>; + pub type Members = StorageValue< + _, + BoundedVec>, T::MaxMembers>, + ValueQuery, + >; /// The current reserved runners-up. /// @@ -633,8 +651,11 @@ pub mod pallet { /// last (i.e. _best_) runner-up will be replaced. #[pallet::storage] #[pallet::getter(fn runners_up)] - pub type RunnersUp = - StorageValue<_, Vec>>, ValueQuery>; + pub type RunnersUp = StorageValue< + _, + BoundedVec>, T::MaxRunnersUp>, + ValueQuery, + >; /// The present candidate list. A current member or runner-up can never enter this vector /// and is always implicitly assumed to be a candidate. @@ -644,7 +665,8 @@ pub mod pallet { /// Invariant: Always sorted based on account id. #[pallet::storage] #[pallet::getter(fn candidates)] - pub type Candidates = StorageValue<_, Vec<(T::AccountId, BalanceOf)>, ValueQuery>; + pub type Candidates = + StorageValue<_, BoundedVec<(T::AccountId, BalanceOf), T::MaxCandidates>, ValueQuery>; /// The total number of vote rounds that have happened, excluding the upcoming one. #[pallet::storage] @@ -656,12 +678,17 @@ pub mod pallet { /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash. #[pallet::storage] #[pallet::getter(fn voting)] - pub type Voting = - StorageMap<_, Twox64Concat, T::AccountId, Voter>, ValueQuery>; + pub type Voting = StorageMap< + _, + Twox64Concat, + T::AccountId, + Voter, T::MaxVoters>, + ValueQuery, + >; #[pallet::genesis_config] pub struct GenesisConfig { - pub members: Vec<(T::AccountId, BalanceOf)>, + pub members: BoundedVec<(T::AccountId, BalanceOf), T::MaxMembers>, } #[cfg(feature = "std")] @@ -697,14 +724,16 @@ pub mod pallet { Ok(_) => { panic!("Duplicate member in elections-phragmen genesis: {}", member) }, - Err(pos) => members.insert( - pos, - SeatHolder { - who: member.clone(), - stake: *stake, - deposit: Zero::zero(), - }, - ), + Err(pos) => members + .try_insert( + pos, + SeatHolder { + who: member.clone(), + stake: *stake, + deposit: Zero::zero(), + }, + ) + .expect("Too many members!"), } }); @@ -779,7 +808,7 @@ impl Pallet { // defensive-only: Members and runners-up are disjoint. This will always be err and // give us an index to insert. if let Err(index) = members.binary_search_by(|m| m.who.cmp(&next_best.who)) { - members.insert(index, next_best.clone()); + members.try_insert(index, next_best.clone()).expect("Too many members!"); } else { // overlap. This can never happen. If so, it seems like our intended replacement // is already a member, so not much more to do. @@ -901,7 +930,9 @@ impl Pallet { 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()); + candidates_and_deposit + .try_append(&mut Self::implicit_candidates_with_deposit()) + .expect("Too many candidates!"); if candidates_and_deposit.len().is_zero() { Self::deposit_event(Event::EmptyTerm); @@ -1069,7 +1100,7 @@ impl Pallet { // 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. - >::put( + >::put::>( new_members_sorted_by_id .iter() .map(|(who, stake)| SeatHolder { @@ -1077,9 +1108,11 @@ impl Pallet { who: who.clone(), stake: *stake, }) - .collect::>(), + .collect::>() + .try_into() + .expect("Too many members!"), ); - >::put( + >::put::>( new_runners_up_sorted_by_rank .into_iter() .map(|(who, stake)| SeatHolder { @@ -1087,7 +1120,9 @@ impl Pallet { who, stake, }) - .collect::>(), + .collect::>() + .try_into() + .expect("Too many runners-up!"), ); // clean candidates. @@ -1136,7 +1171,7 @@ impl SortedMembers for Pallet { stake: Default::default(), deposit: Default::default(), }; - members.insert(pos, s) + members.try_insert(pos, s).expect("Too many members.") }, }) } @@ -1223,6 +1258,8 @@ mod tests { pub static VotingBondFactor: u64 = 0; pub static CandidacyBond: u64 = 3; pub static DesiredMembers: u32 = 2; + pub static MaxMembers: u32 = 50; + pub static MaxRunnersUp: u32 = 30; pub static DesiredRunnersUp: u32 = 0; pub static TermDuration: u64 = 5; pub static Members: Vec = vec![]; @@ -1296,6 +1333,8 @@ mod tests { type KickedMember = (); type WeightInfo = (); type MaxVoters = PhragmenMaxVoters; + type MaxMembers = MaxMembers; + type MaxRunnersUp = MaxRunnersUp; type MaxCandidates = PhragmenMaxCandidates; } @@ -1373,7 +1412,7 @@ mod tests { ], }, elections: elections_phragmen::GenesisConfig:: { - members: self.genesis_members, + members: self.genesis_members.try_into().unwrap(), }, } .build_storage() From 82e9f072386b1b108d41e3e095ca466f9061d1b2 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 24 Oct 2022 15:16:56 +0200 Subject: [PATCH 02/34] fixes --- frame/elections-phragmen/src/lib.rs | 33 +++++++++++++++++------------ 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 685e7e2c26114..b194026211944 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -98,7 +98,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -use codec::{Decode, Encode}; +use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ bounded_vec, traits::{ @@ -145,10 +145,10 @@ pub enum Renouncing { } /// An active voter. -#[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, TypeInfo)] -pub struct Voter { +#[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, TypeInfo, MaxEncodedLen)] +pub struct Voter> { /// The members being backed. - pub votes: BoundedVec, + pub votes: BoundedVec, /// The amount of stake placed on this vote. pub stake: Balance, /// The amount of deposit reserved for this vote. @@ -157,16 +157,20 @@ pub struct Voter { pub deposit: Balance, } -impl> Default - for Voter +impl> Default + for Voter { fn default() -> Self { - Self { votes: bounded_vec![], stake: Default::default(), deposit: Default::default() } + Self { + votes: BoundedVec::default(), + stake: Default::default(), + deposit: Default::default(), + } } } /// A holder of a seat as either a member or a runner-up. -#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq, TypeInfo)] +#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq, TypeInfo, MaxEncodedLen)] pub struct SeatHolder { /// The holder. pub who: AccountId, @@ -194,7 +198,6 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] #[pallet::storage_version(STORAGE_VERSION)] - #[pallet::without_storage_info] pub struct Pallet(PhantomData); #[pallet::config] @@ -260,7 +263,7 @@ pub mod pallet { /// the size of the election. When this limit is reached no more /// candidates are accepted in the election. #[pallet::constant] - type MaxCandidates: Get; + type MaxCandidates: Get + TypeInfo; /// The maximum number of voters to allow in a phragmen election. /// @@ -328,7 +331,7 @@ pub mod pallet { )] pub fn vote( origin: OriginFor, - votes: Vec, + votes: BoundedVec, #[pallet::compact] value: BalanceOf, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; @@ -682,7 +685,7 @@ pub mod pallet { _, Twox64Concat, T::AccountId, - Voter, T::MaxVoters>, + Voter, T::MaxCandidates>, ValueQuery, >; @@ -743,7 +746,11 @@ pub mod pallet { // remove_lock is noop if lock is not there. >::insert( &member, - Voter { votes: vec![member.clone()], stake: *stake, deposit: Zero::zero() }, + Voter { + votes: bounded_vec![member.clone()], + stake: *stake, + deposit: Zero::zero(), + }, ); member.clone() From 03148fe0e222c8702eb89841d93bd0230642c6dc Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 25 Oct 2022 15:45:13 +0200 Subject: [PATCH 03/34] passing tests --- frame/elections-phragmen/src/lib.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index b194026211944..a1610a1c12dfe 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1320,9 +1320,18 @@ mod tests { parameter_types! { pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect"; pub const PhragmenMaxVoters: u32 = 1000; + #[derive(PartialEq, Debug)] pub const PhragmenMaxCandidates: u32 = 100; } + impl TypeInfo for PhragmenMaxCandidates { + type Identity = Self; + + fn type_info() -> scale_info::Type { + u32::type_info() + } + } + impl Config for Test { type PalletId = ElectionsPhragmenPalletId; type RuntimeEvent = RuntimeEvent; @@ -1543,10 +1552,10 @@ mod tests { // call was changing. Currently it is a wrapper for the original call and does not do much. // Nonetheless, totally harmless. ensure_signed(origin.clone()).expect("vote origin must be signed"); - Elections::vote(origin, votes, stake) + Elections::vote(origin, votes.try_into().unwrap(), stake) } - fn votes_of(who: &u64) -> Vec { + fn votes_of(who: &u64) -> BoundedVec::MaxCandidates> { Voting::::get(who).votes } @@ -1589,11 +1598,11 @@ mod tests { assert_eq!( Elections::voting(1), - Voter { stake: 10u64, votes: vec![1], deposit: 0 } + Voter { stake: 10u64, votes: vec![1].try_into().unwrap(), deposit: 0 } ); assert_eq!( Elections::voting(2), - Voter { stake: 20u64, votes: vec![2], deposit: 0 } + Voter { stake: 20u64, votes: vec![2].try_into().unwrap(), deposit: 0 } ); // they will persist since they have self vote. @@ -1613,11 +1622,11 @@ mod tests { assert_eq!( Elections::voting(1), - Voter { stake: 10u64, votes: vec![1], deposit: 0 } + Voter { stake: 10u64, votes: vec![1].try_into().unwrap(), deposit: 0 } ); assert_eq!( Elections::voting(2), - Voter { stake: 20u64, votes: vec![2], deposit: 0 } + Voter { stake: 20u64, votes: vec![2].try_into().unwrap(), deposit: 0 } ); assert_ok!(Elections::remove_voter(RuntimeOrigin::signed(1))); @@ -1644,11 +1653,11 @@ mod tests { assert_eq!( Elections::voting(1), - Voter { stake: 10u64, votes: vec![1], deposit: 0 } + Voter { stake: 10u64, votes: vec![1].try_into().unwrap(), deposit: 0 } ); assert_eq!( Elections::voting(2), - Voter { stake: 20u64, votes: vec![2], deposit: 0 } + Voter { stake: 20u64, votes: vec![2].try_into().unwrap(), deposit: 0 } ); // they will persist since they have self vote. From f61ea3233b846563d034b5e715d113c4d6b027ab Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 25 Oct 2022 17:27:34 +0200 Subject: [PATCH 04/34] map_err --- frame/elections-phragmen/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index a1610a1c12dfe..219b89ac88465 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -431,10 +431,11 @@ pub mod pallet { T::Currency::reserve(&who, T::CandidacyBond::get()) .map_err(|_| Error::::InsufficientCandidateFunds)?; - >::mutate(|c| { + >::mutate(|c| -> Result<(), DispatchError> { c.try_insert(index, (who, T::CandidacyBond::get())) - .expect("Too many candidates!") - }); + .map_err(|_| Error::::TooManyCandidates)?; + Ok(()) + })?; Ok(()) } From f6ec1e7f43c6ed45c64c728af46365b326f10bed Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 25 Oct 2022 22:49:38 +0200 Subject: [PATCH 05/34] define bounds in runtime --- bin/node/runtime/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 5ba89e6595e72..1e2fa38b9cddd 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -992,6 +992,8 @@ parameter_types! { pub const DesiredRunnersUp: u32 = 7; pub const MaxVoters: u32 = 10 * 1000; pub const MaxCandidates: u32 = 1000; + pub const MaxMembers: u32 = 20; + pub const MaxRunnersUp: u32 = 15; pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect"; } @@ -1017,6 +1019,8 @@ impl pallet_elections_phragmen::Config for Runtime { type TermDuration = TermDuration; type MaxVoters = MaxVoters; type MaxCandidates = MaxCandidates; + type MaxMembers = MaxMembers; + type MaxRunnersUp = MaxRunnersUp; type WeightInfo = pallet_elections_phragmen::weights::SubstrateWeight; } From eae55fa0d6039a149b81f8e79d39670d6200f373 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 26 Oct 2022 17:17:59 +0200 Subject: [PATCH 06/34] fix CI --- bin/node/runtime/src/lib.rs | 8 ++++++++ frame/elections-phragmen/src/lib.rs | 3 +-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 1e2fa38b9cddd..85f714dcb94ef 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -997,6 +997,14 @@ parameter_types! { pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect"; } +impl scale_info::TypeInfo for MaxCandidates { + type Identity = Self; + + fn type_info() -> scale_info::Type { + u32::type_info() + } +} + // Make sure that there are no more than `MaxMembers` members elected via elections-phragmen. const_assert!(DesiredMembers::get() <= CouncilMaxMembers::get()); diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 219b89ac88465..81835fc4a36ca 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -100,7 +100,6 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ - bounded_vec, traits::{ defensive_prelude::*, ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, @@ -748,7 +747,7 @@ pub mod pallet { >::insert( &member, Voter { - votes: bounded_vec![member.clone()], + votes: vec![member.clone()].try_into().expect("Too many votes."), stake: *stake, deposit: Zero::zero(), }, From 5e65bfa66c394fb32498f9fb2ba2eaeb03e90908 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 26 Oct 2022 17:28:52 +0200 Subject: [PATCH 07/34] fix --- bin/node/cli/src/chain_spec.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 8d74f2bde0f44..cc120fdd05cb8 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -327,7 +327,9 @@ pub fn testnet_genesis( .take((num_endowed_accounts + 1) / 2) .cloned() .map(|member| (member, STASH)) - .collect(), + .collect() + .try_into() + .expect("Too many members."), }, council: CouncilConfig::default(), technical_committee: TechnicalCommitteeConfig { From a201e5a2cf0e6fa2061d437305a42397da091108 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 26 Oct 2022 17:46:23 +0200 Subject: [PATCH 08/34] .. --- bin/node/cli/src/chain_spec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index cc120fdd05cb8..9da8d8c760ad0 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -327,7 +327,7 @@ pub fn testnet_genesis( .take((num_endowed_accounts + 1) / 2) .cloned() .map(|member| (member, STASH)) - .collect() + .collect::>() .try_into() .expect("Too many members."), }, From 619a707611e6997326531b94233b422769605603 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 26 Oct 2022 17:59:26 +0200 Subject: [PATCH 09/34] update benchmarks --- frame/elections-phragmen/src/benchmarking.rs | 24 ++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index 06ac8d7c60162..59089fd711b8c 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -84,9 +84,9 @@ fn submit_candidates_with_self_vote( ) -> Result, &'static str> { let candidates = submit_candidates::(c, prefix)?; let stake = default_stake::(c); - let _ = candidates - .iter() - .try_for_each(|c| submit_voter::(c.clone(), vec![c.clone()], stake).map(|_| ()))?; + let _ = candidates.iter().try_for_each(|c| { + submit_voter::(c.clone(), vec![c.clone()].try_into().unwrap(), stake).map(|_| ()) + })?; Ok(candidates) } @@ -96,7 +96,7 @@ fn submit_voter( votes: Vec, stake: BalanceOf, ) -> DispatchResultWithPostInfo { - >::vote(RawOrigin::Signed(caller).into(), votes, stake) + >::vote(RawOrigin::Signed(caller).into(), votes.try_into().unwrap(), stake) } /// create `num_voter` voters who randomly vote for at most `votes` of `all_candidates` if @@ -110,7 +110,13 @@ fn distribute_voters( for i in 0..num_voters { // to ensure that votes are different all_candidates.rotate_left(1); - let votes = all_candidates.iter().cloned().take(votes).collect::>(); + let votes = all_candidates + .iter() + .cloned() + .take(votes) + .collect::>() + .try_into() + .unwrap(); let voter = endowed_account::("voter", i); submit_voter::(voter, votes, stake)?; } @@ -165,7 +171,7 @@ benchmarks! { votes.rotate_left(1); whitelist!(caller); - }: vote(RawOrigin::Signed(caller), votes, stake) + }: vote(RawOrigin::Signed(caller), votes.try_into().unwrap(), stake) vote_more { let v in 2 .. (MAXIMUM_VOTE as u32); @@ -187,7 +193,7 @@ benchmarks! { assert!(votes.len() > >::get(caller.clone()).votes.len()); whitelist!(caller); - }: vote(RawOrigin::Signed(caller), votes, stake / >::from(10u32)) + }: vote(RawOrigin::Signed(caller), votes.try_into().unwrap(), stake / >::from(10u32)) vote_less { let v in 2 .. (MAXIMUM_VOTE as u32); @@ -208,7 +214,7 @@ benchmarks! { assert!(votes.len() < >::get(caller.clone()).votes.len()); whitelist!(caller); - }: vote(RawOrigin::Signed(caller), votes, stake) + }: vote(RawOrigin::Signed(caller), votes.try_into().unwrap(), stake) remove_voter { // we fix the number of voted candidates to max @@ -228,7 +234,7 @@ benchmarks! { submit_candidacy { // number of already existing candidates. - let c in 1 .. T::MaxCandidates::get(); + let c in 1 .. T::MaxCandidates::get()-1; // we fix the number of members to the number of desired members and runners-up. We'll be in // this state almost always. let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); From 0589f640b47f63580e78bc91acc8545650569bde Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 29 Oct 2022 13:25:48 +0200 Subject: [PATCH 10/34] fixes --- bin/node/cli/src/chain_spec.rs | 4 ++- bin/node/runtime/src/lib.rs | 4 --- frame/elections-phragmen/src/lib.rs | 40 +++++++++++++---------------- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 9da8d8c760ad0..9850a62bbee86 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -329,7 +329,9 @@ pub fn testnet_genesis( .map(|member| (member, STASH)) .collect::>() .try_into() - .expect("Too many members."), + .expect( + "Elections-Phragmen: Cannot accept more than DesiredMembers genesis member.", + ), }, council: CouncilConfig::default(), technical_committee: TechnicalCommitteeConfig { diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 3e66889025776..6cac751af2826 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -992,8 +992,6 @@ parameter_types! { pub const DesiredRunnersUp: u32 = 7; pub const MaxVoters: u32 = 10 * 1000; pub const MaxCandidates: u32 = 1000; - pub const MaxMembers: u32 = 20; - pub const MaxRunnersUp: u32 = 15; pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect"; } @@ -1027,8 +1025,6 @@ impl pallet_elections_phragmen::Config for Runtime { type TermDuration = TermDuration; type MaxVoters = MaxVoters; type MaxCandidates = MaxCandidates; - type MaxMembers = MaxMembers; - type MaxRunnersUp = MaxRunnersUp; type WeightInfo = pallet_elections_phragmen::weights::SubstrateWeight; } diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 81835fc4a36ca..58b96c83a7589 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -271,14 +271,6 @@ pub mod pallet { #[pallet::constant] type MaxVoters: Get; - /// The maximum number of elected members. - #[pallet::constant] - type MaxMembers: Get; - - /// The maximum number of runners-up. - #[pallet::constant] - type MaxRunnersUp: Get; - /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -644,7 +636,7 @@ pub mod pallet { #[pallet::getter(fn members)] pub type Members = StorageValue< _, - BoundedVec>, T::MaxMembers>, + BoundedVec>, T::DesiredMembers>, ValueQuery, >; @@ -656,7 +648,7 @@ pub mod pallet { #[pallet::getter(fn runners_up)] pub type RunnersUp = StorageValue< _, - BoundedVec>, T::MaxRunnersUp>, + BoundedVec>, T::DesiredRunnersUp>, ValueQuery, >; @@ -691,7 +683,7 @@ pub mod pallet { #[pallet::genesis_config] pub struct GenesisConfig { - pub members: BoundedVec<(T::AccountId, BalanceOf), T::MaxMembers>, + pub members: BoundedVec<(T::AccountId, BalanceOf), T::DesiredMembers>, } #[cfg(feature = "std")] @@ -736,7 +728,7 @@ pub mod pallet { deposit: Zero::zero(), }, ) - .expect("Too many members!"), + .expect("Cannot accept more than DesiredMembers genesis member."), } }); @@ -815,7 +807,9 @@ impl Pallet { // defensive-only: Members and runners-up are disjoint. This will always be err and // give us an index to insert. if let Err(index) = members.binary_search_by(|m| m.who.cmp(&next_best.who)) { - members.try_insert(index, next_best.clone()).expect("Too many members!"); + members + .try_insert(index, next_best.clone()) + .expect("Cannot accept more than DesiredMembers genesis member."); } else { // overlap. This can never happen. If so, it seems like our intended replacement // is already a member, so not much more to do. @@ -1107,7 +1101,7 @@ impl Pallet { // 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. - >::put::>( + >::put::>( new_members_sorted_by_id .iter() .map(|(who, stake)| SeatHolder { @@ -1117,9 +1111,9 @@ impl Pallet { }) .collect::>() .try_into() - .expect("Too many members!"), + .expect("Cannot accept more than DesiredMembers genesis member."), ); - >::put::>( + >::put::>( new_runners_up_sorted_by_rank .into_iter() .map(|(who, stake)| SeatHolder { @@ -1178,7 +1172,9 @@ impl SortedMembers for Pallet { stake: Default::default(), deposit: Default::default(), }; - members.try_insert(pos, s).expect("Too many members.") + members + .try_insert(pos, s) + .expect("Cannot accept more than DesiredMembers genesis member.") }, }) } @@ -1265,8 +1261,6 @@ mod tests { pub static VotingBondFactor: u64 = 0; pub static CandidacyBond: u64 = 3; pub static DesiredMembers: u32 = 2; - pub static MaxMembers: u32 = 50; - pub static MaxRunnersUp: u32 = 30; pub static DesiredRunnersUp: u32 = 0; pub static TermDuration: u64 = 5; pub static Members: Vec = vec![]; @@ -1349,8 +1343,6 @@ mod tests { type KickedMember = (); type WeightInfo = (); type MaxVoters = PhragmenMaxVoters; - type MaxMembers = MaxMembers; - type MaxRunnersUp = MaxRunnersUp; type MaxCandidates = PhragmenMaxCandidates; } @@ -1428,7 +1420,10 @@ mod tests { ], }, elections: elections_phragmen::GenesisConfig:: { - members: self.genesis_members.try_into().unwrap(), + members: self + .genesis_members + .try_into() + .expect("Cannot accept more than DesiredMembers genesis member."), }, } .build_storage() @@ -1686,6 +1681,7 @@ mod tests { .build_and_execute(|| {}); } + // #[test] #[should_panic = "Cannot accept more than DesiredMembers genesis member"] fn genesis_members_cannot_too_many() { From 38d1c01358d7e7ffe0a7abba449faaacf26d936a Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 29 Oct 2022 13:33:10 +0200 Subject: [PATCH 11/34] error messages updated --- frame/elections-phragmen/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 58b96c83a7589..5c9fd33e5bf8d 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -739,7 +739,9 @@ pub mod pallet { >::insert( &member, Voter { - votes: vec![member.clone()].try_into().expect("Too many votes."), + votes: vec![member.clone()] + .try_into() + .expect("Cannot expect more votes than there are candidates."), stake: *stake, deposit: Zero::zero(), }, @@ -809,7 +811,7 @@ impl Pallet { if let Err(index) = members.binary_search_by(|m| m.who.cmp(&next_best.who)) { members .try_insert(index, next_best.clone()) - .expect("Cannot accept more than DesiredMembers genesis member."); + .expect("Cannot accept more than DesiredMembers."); } else { // overlap. This can never happen. If so, it seems like our intended replacement // is already a member, so not much more to do. @@ -933,7 +935,7 @@ impl Pallet { // add all the previous members and runners-up as candidates as well. candidates_and_deposit .try_append(&mut Self::implicit_candidates_with_deposit()) - .expect("Too many candidates!"); + .expect("Cannot accept more than MaxCandidates candidates."); if candidates_and_deposit.len().is_zero() { Self::deposit_event(Event::EmptyTerm); @@ -1111,7 +1113,7 @@ impl Pallet { }) .collect::>() .try_into() - .expect("Cannot accept more than DesiredMembers genesis member."), + .expect("Cannot accept more than DesiredMembers."), ); >::put::>( new_runners_up_sorted_by_rank @@ -1123,7 +1125,7 @@ impl Pallet { }) .collect::>() .try_into() - .expect("Too many runners-up!"), + .expect("Cannot accept more than DesiredRunnersUp."), ); // clean candidates. @@ -1172,9 +1174,7 @@ impl SortedMembers for Pallet { stake: Default::default(), deposit: Default::default(), }; - members - .try_insert(pos, s) - .expect("Cannot accept more than DesiredMembers genesis member.") + members.try_insert(pos, s).expect("Cannot accept more than DesiredMembers.") }, }) } From 872c176ffc5b582dfad7c3a3286c7b16073d54c9 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Wed, 2 Nov 2022 11:47:48 +0100 Subject: [PATCH 12/34] Update frame/elections-phragmen/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- frame/elections-phragmen/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 5c9fd33e5bf8d..0ddd6fb3cd8aa 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -935,7 +935,14 @@ impl Pallet { // add all the previous members and runners-up as candidates as well. candidates_and_deposit .try_append(&mut Self::implicit_candidates_with_deposit()) - .expect("Cannot accept more than MaxCandidates candidates."); + match candidates_and_deposit + .try_append(&mut Self::implicit_candidates_with_deposit()) + .map_err(|_| T::DbWeight::get().reads(3)) { + Ok(_) => (), + Err(weight) => return weight, + } + + if candidates_and_deposit.len().is_zero() { Self::deposit_event(Event::EmptyTerm); From c15724928e657bee3ea534417eada62f98e9c42e Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 2 Nov 2022 11:57:13 +0100 Subject: [PATCH 13/34] fix --- frame/elections-phragmen/src/lib.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 0ddd6fb3cd8aa..1d951231ecd4e 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -933,16 +933,13 @@ impl Pallet { let mut candidates_and_deposit = Self::candidates(); // add all the previous members and runners-up as candidates as well. - candidates_and_deposit - .try_append(&mut Self::implicit_candidates_with_deposit()) match candidates_and_deposit .try_append(&mut Self::implicit_candidates_with_deposit()) - .map_err(|_| T::DbWeight::get().reads(3)) { - Ok(_) => (), - Err(weight) => return weight, - } - - + .map_err(|_| T::DbWeight::get().reads(3)) + { + Ok(_) => (), + Err(weight) => return weight, + } if candidates_and_deposit.len().is_zero() { Self::deposit_event(Event::EmptyTerm); From f06dc4ca3dd6f5820f236b8fa650d667acaa5798 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 2 Nov 2022 23:14:05 +0100 Subject: [PATCH 14/34] remove TypeInfo --- bin/node/runtime/src/lib.rs | 8 ----- frame/elections-phragmen/src/benchmarking.rs | 2 +- frame/elections-phragmen/src/lib.rs | 35 +++++--------------- 3 files changed, 10 insertions(+), 35 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 1f48049b0ef3e..d49312bd6fe3f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -995,14 +995,6 @@ parameter_types! { pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect"; } -impl scale_info::TypeInfo for MaxCandidates { - type Identity = Self; - - fn type_info() -> scale_info::Type { - u32::type_info() - } -} - // Make sure that there are no more than `MaxMembers` members elected via elections-phragmen. const_assert!(DesiredMembers::get() <= CouncilMaxMembers::get()); diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index 59089fd711b8c..66cf957ea777d 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -234,7 +234,7 @@ benchmarks! { submit_candidacy { // number of already existing candidates. - let c in 1 .. T::MaxCandidates::get()-1; + let c in 1 .. T::MaxCandidates::get() - 1; // we fix the number of members to the number of desired members and runners-up. We'll be in // this state almost always. let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 1d951231ecd4e..0003608c9ca74 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -101,7 +101,7 @@ use codec::{Decode, Encode, MaxEncodedLen}; 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, }, @@ -145,9 +145,9 @@ pub enum Renouncing { /// An active voter. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, TypeInfo, MaxEncodedLen)] -pub struct Voter> { +pub struct Voter { /// The members being backed. - pub votes: BoundedVec, + pub votes: BoundedVec>, /// The amount of stake placed on this vote. pub stake: Balance, /// The amount of deposit reserved for this vote. @@ -156,9 +156,7 @@ pub struct Voter> { pub deposit: Balance, } -impl> Default - for Voter -{ +impl Default for Voter { fn default() -> Self { Self { votes: BoundedVec::default(), @@ -262,7 +260,7 @@ pub mod pallet { /// the size of the election. When this limit is reached no more /// candidates are accepted in the election. #[pallet::constant] - type MaxCandidates: Get + TypeInfo; + type MaxCandidates: Get; /// The maximum number of voters to allow in a phragmen election. /// @@ -322,7 +320,7 @@ pub mod pallet { )] pub fn vote( origin: OriginFor, - votes: BoundedVec, + votes: BoundedVec>, #[pallet::compact] value: BalanceOf, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; @@ -673,13 +671,8 @@ pub mod pallet { /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash. #[pallet::storage] #[pallet::getter(fn voting)] - pub type Voting = StorageMap< - _, - Twox64Concat, - T::AccountId, - Voter, T::MaxCandidates>, - ValueQuery, - >; + pub type Voting = + StorageMap<_, Twox64Concat, T::AccountId, Voter>, ValueQuery>; #[pallet::genesis_config] pub struct GenesisConfig { @@ -1318,18 +1311,9 @@ mod tests { parameter_types! { pub const ElectionsPhragmenPalletId: LockIdentifier = *b"phrelect"; pub const PhragmenMaxVoters: u32 = 1000; - #[derive(PartialEq, Debug)] pub const PhragmenMaxCandidates: u32 = 100; } - impl TypeInfo for PhragmenMaxCandidates { - type Identity = Self; - - fn type_info() -> scale_info::Type { - u32::type_info() - } - } - impl Config for Test { type PalletId = ElectionsPhragmenPalletId; type RuntimeEvent = RuntimeEvent; @@ -1554,7 +1538,7 @@ mod tests { Elections::vote(origin, votes.try_into().unwrap(), stake) } - fn votes_of(who: &u64) -> BoundedVec::MaxCandidates> { + fn votes_of(who: &u64) -> BoundedVec> { Voting::::get(who).votes } @@ -1685,7 +1669,6 @@ mod tests { .build_and_execute(|| {}); } - // #[test] #[should_panic = "Cannot accept more than DesiredMembers genesis member"] fn genesis_members_cannot_too_many() { From 29604026dd5eab7e82612fb630f889f817569054 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 4 Nov 2022 18:10:01 +0100 Subject: [PATCH 15/34] update --- frame/elections-phragmen/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index 66cf957ea777d..19a41c59bbf2b 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -234,7 +234,7 @@ benchmarks! { submit_candidacy { // number of already existing candidates. - let c in 1 .. T::MaxCandidates::get() - 1; + let c in 1 .. T::MaxCandidates::get(); // we fix the number of members to the number of desired members and runners-up. We'll be in // this state almost always. let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); From 1afa5b325d8c9fca7f7ada58d7b07d1dd1a34dc1 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 5 Nov 2022 16:01:55 +0100 Subject: [PATCH 16/34] fix --- frame/elections-phragmen/src/benchmarking.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index 19a41c59bbf2b..e07be4cd87409 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -245,9 +245,6 @@ benchmarks! { // create m members and runners combined. let _ = fill_seats_up_to::(m)?; - // create previous candidates; - let _ = submit_candidates::(c, "candidates")?; - // we assume worse case that: extrinsic is successful and candidate is not duplicate. let candidate_account = endowed_account::("caller", 0); whitelist!(candidate_account); From bc93503bf2499169f1729b7f83d9c661d332aba0 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sun, 6 Nov 2022 09:08:40 +0100 Subject: [PATCH 17/34] handle error --- frame/elections-phragmen/src/lib.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 0003608c9ca74..39ba804154a3a 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1110,7 +1110,13 @@ impl Pallet { }) .collect::>() .try_into() - .expect("Cannot accept more than DesiredMembers."), + .unwrap_or_else(|_| { + log::error!( + target: "runtime::elections-phragmen", + "There are too many members.", + ); + Default::default() + }), ); >::put::>( new_runners_up_sorted_by_rank @@ -1122,7 +1128,13 @@ impl Pallet { }) .collect::>() .try_into() - .expect("Cannot accept more than DesiredRunnersUp."), + .unwrap_or_else(|_| { + log::error!( + target: "runtime::elections-phragmen", + "There are too many runners up.", + ); + Default::default() + }), ); // clean candidates. From b5ee15d6e97b53e9fcfc2e396c6539a9b71d1cda Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sun, 6 Nov 2022 09:38:12 +0100 Subject: [PATCH 18/34] fix logic --- frame/elections-phragmen/src/lib.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 39ba804154a3a..e115a92488a17 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1103,6 +1103,7 @@ impl Pallet { >::put::>( new_members_sorted_by_id .iter() + .take(T::DesiredMembers::get() as usize) .map(|(who, stake)| SeatHolder { deposit: deposit_of_candidate(who), who: who.clone(), @@ -1110,31 +1111,20 @@ impl Pallet { }) .collect::>() .try_into() - .unwrap_or_else(|_| { - log::error!( - target: "runtime::elections-phragmen", - "There are too many members.", - ); - Default::default() - }), + .defensive_unwrap_or_default(), ); >::put::>( new_runners_up_sorted_by_rank .into_iter() + .take(T::DesiredRunnersUp::get() as usize) .map(|(who, stake)| SeatHolder { deposit: deposit_of_candidate(&who), - who, - stake, + who: who.clone(), + stake: stake.clone(), }) .collect::>() .try_into() - .unwrap_or_else(|_| { - log::error!( - target: "runtime::elections-phragmen", - "There are too many runners up.", - ); - Default::default() - }), + .defensive_unwrap_or_default(), ); // clean candidates. From 0b2c12e3fd771c7375f6425eb202e77e0e024d20 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sun, 6 Nov 2022 10:49:54 +0100 Subject: [PATCH 19/34] make clippy happy --- frame/elections-phragmen/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index e115a92488a17..689a06d0816cd 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1119,8 +1119,8 @@ impl Pallet { .take(T::DesiredRunnersUp::get() as usize) .map(|(who, stake)| SeatHolder { deposit: deposit_of_candidate(&who), - who: who.clone(), - stake: stake.clone(), + who, + stake, }) .collect::>() .try_into() From e737539312c9bd22a9074d595fa5ffa7ec75b173 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 8 Nov 2022 12:11:32 +0100 Subject: [PATCH 20/34] nit fix --- frame/elections-phragmen/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 689a06d0816cd..f05726f9c283f 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -424,8 +424,7 @@ pub mod pallet { c.try_insert(index, (who, T::CandidacyBond::get())) .map_err(|_| Error::::TooManyCandidates)?; Ok(()) - })?; - Ok(()) + }) } /// Renounce one's intention to be a candidate for the next election round. 3 potential From 6f3f2ee8127a98023552fedf02eeb76a8aa17358 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Fri, 11 Nov 2022 20:05:51 +0100 Subject: [PATCH 21/34] remove redundent checks --- frame/elections-phragmen/src/lib.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index f05726f9c283f..5641fde0513f9 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -325,8 +325,6 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { 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.is_empty(), Error::::NoVotes); let candidates_count = >::decode_len().unwrap_or(0); @@ -407,10 +405,6 @@ pub mod pallet { let actual_count = >::decode_len().unwrap_or(0) as u32; ensure!(actual_count <= candidate_count, Error::::InvalidWitnessData); - ensure!( - actual_count <= ::MaxCandidates::get(), - Error::::TooManyCandidates - ); let index = Self::is_candidate(&who).err().ok_or(Error::::DuplicatedCandidate)?; From 04c5a828cda25ae998fde90da187b80f67ab5d60 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sat, 12 Nov 2022 08:57:34 +0100 Subject: [PATCH 22/34] fix --- frame/elections-phragmen/src/benchmarking.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/benchmarking.rs b/frame/elections-phragmen/src/benchmarking.rs index e07be4cd87409..66cf957ea777d 100644 --- a/frame/elections-phragmen/src/benchmarking.rs +++ b/frame/elections-phragmen/src/benchmarking.rs @@ -234,7 +234,7 @@ benchmarks! { submit_candidacy { // number of already existing candidates. - let c in 1 .. T::MaxCandidates::get(); + let c in 1 .. T::MaxCandidates::get() - 1; // we fix the number of members to the number of desired members and runners-up. We'll be in // this state almost always. let m = T::DesiredMembers::get() + T::DesiredRunnersUp::get(); @@ -245,6 +245,9 @@ benchmarks! { // create m members and runners combined. let _ = fill_seats_up_to::(m)?; + // create previous candidates; + let _ = submit_candidates::(c, "candidates")?; + // we assume worse case that: extrinsic is successful and candidate is not duplicate. let candidate_account = endowed_account::("caller", 0); whitelist!(candidate_account); From 076671a9fefcca53e52a5fe71a0f5edaf97e6698 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Thu, 24 Nov 2022 14:37:34 +0100 Subject: [PATCH 23/34] Update frame/elections-phragmen/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- frame/elections-phragmen/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 5641fde0513f9..6c98a588bb8c7 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1104,7 +1104,7 @@ impl Pallet { }) .collect::>() .try_into() - .defensive_unwrap_or_default(), + .expect("number members will never exceed T::DesiredMembers. qed."), ); >::put::>( new_runners_up_sorted_by_rank From 0ad62c1ffb45510899de63ef38d78908d6857adb Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Thu, 24 Nov 2022 14:37:48 +0100 Subject: [PATCH 24/34] Update frame/elections-phragmen/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Gonçalo Pestana --- frame/elections-phragmen/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 6c98a588bb8c7..b14b020f66eb5 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1117,7 +1117,7 @@ impl Pallet { }) .collect::>() .try_into() - .defensive_unwrap_or_default(), + .expect("number runners up will never exceed T::DesiredRunnersUp. qed."), ); // clean candidates. From ff0281fd61e050ee4a89d1fc93b3ee3bbb041f33 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Fri, 25 Nov 2022 20:43:50 +0100 Subject: [PATCH 25/34] migration --- frame/elections-phragmen/src/lib.rs | 4 +- .../elections-phragmen/src/migrations/mod.rs | 2 + frame/elections-phragmen/src/migrations/v6.rs | 44 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 frame/elections-phragmen/src/migrations/v6.rs diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index b14b020f66eb5..93a3626ac435b 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1117,7 +1117,9 @@ impl Pallet { }) .collect::>() .try_into() - .expect("number runners up will never exceed T::DesiredRunnersUp. qed."), + .expect( + "number runners up will never exceed T::DesiredRunnersUp. qed.", + ), ); // clean candidates. diff --git a/frame/elections-phragmen/src/migrations/mod.rs b/frame/elections-phragmen/src/migrations/mod.rs index 7c62e8fa93067..4b907e676675b 100644 --- a/frame/elections-phragmen/src/migrations/mod.rs +++ b/frame/elections-phragmen/src/migrations/mod.rs @@ -23,3 +23,5 @@ pub mod v3; pub mod v4; /// Version 5. pub mod v5; +/// Version 6. +pub mod v6; diff --git a/frame/elections-phragmen/src/migrations/v6.rs b/frame/elections-phragmen/src/migrations/v6.rs new file mode 100644 index 0000000000000..6ba7ee04b452f --- /dev/null +++ b/frame/elections-phragmen/src/migrations/v6.rs @@ -0,0 +1,44 @@ +// This file is part of Substrate. + +// Copyright (C) 2021-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use crate::{BalanceOf, Config, Pallet, MAXIMUM_VOTE}; +use codec::{Decode, Encode}; +use frame_support::{traits::ConstU32, BoundedVec, RuntimeDebug, Twox64Concat}; +use sp_std::prelude::*; + +#[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] +struct DeprecatedVoter { + votes: Vec, + stake: Balance, + deposit: Balance, +} + +#[frame_support::storage_alias] +type Voting = StorageMap< + Pallet, + Twox64Concat, + ::AccountId, + crate::Voter<::AccountId, BalanceOf>, +>; + +pub fn migrate_voting() { + Voting::::translate::>, _>(|_, voter| { + let bounded_votes: BoundedVec> = + voter.votes.try_into().unwrap(); + Some(crate::Voter { votes: bounded_votes, stake: voter.stake, deposit: voter.deposit }) + }); +} From 359cac173b13cbb8270683542c1fb38386e47f2d Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sun, 27 Nov 2022 10:22:56 +0100 Subject: [PATCH 26/34] complete migrations --- frame/elections-phragmen/src/migrations/v6.rs | 101 ++++++++++++++++-- 1 file changed, 90 insertions(+), 11 deletions(-) diff --git a/frame/elections-phragmen/src/migrations/v6.rs b/frame/elections-phragmen/src/migrations/v6.rs index 6ba7ee04b452f..d13c18ca0eb16 100644 --- a/frame/elections-phragmen/src/migrations/v6.rs +++ b/frame/elections-phragmen/src/migrations/v6.rs @@ -15,9 +15,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{BalanceOf, Config, Pallet, MAXIMUM_VOTE}; +use crate::{BalanceOf, Config, Pallet, SeatHolder, Weight, MAXIMUM_VOTE}; use codec::{Decode, Encode}; -use frame_support::{traits::ConstU32, BoundedVec, RuntimeDebug, Twox64Concat}; +use frame_support::{pallet_prelude::StorageVersion, traits::ConstU32, BoundedVec, RuntimeDebug}; +use sp_core::Get; +use sp_runtime::Saturating; use sp_std::prelude::*; #[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] @@ -27,18 +29,95 @@ struct DeprecatedVoter { deposit: Balance, } -#[frame_support::storage_alias] -type Voting = StorageMap< - Pallet, - Twox64Concat, - ::AccountId, - crate::Voter<::AccountId, BalanceOf>, ->; +pub fn apply() -> Weight { + let storage_version = StorageVersion::get::>(); + log::info!( + target: "runtime::elections-phragmen", + "Running migration for elections-phragmen with storage version {:?}", + storage_version, + ); -pub fn migrate_voting() { - Voting::::translate::>, _>(|_, voter| { + if storage_version <= 5 { + let weight = migrate_voting::(); + weight.saturating_add(migrate_members::()); + weight.saturating_add(migrate_runners_up::()); + weight.saturating_add(migrate_candidates::()); + + StorageVersion::new(6).put::>(); + + weight + } else { + log::warn!( + target: "runtime::elections-phragmen", + "Attempted to apply migration to V6 but failed because storage version is {:?}", + storage_version, + ); + Weight::zero() + } +} + +pub fn migrate_voting() -> Weight { + let mut translated = 0u64; + crate::Voting::::translate::>, _>(|_, voter| { + translated.saturating_inc(); let bounded_votes: BoundedVec> = voter.votes.try_into().unwrap(); Some(crate::Voter { votes: bounded_votes, stake: voter.stake, deposit: voter.deposit }) }); + T::DbWeight::get().reads_writes(translated, translated) +} + +pub fn migrate_members() -> Weight { + let mut translated = 0u64; + let _ = crate::Members::::translate::>>, _>( + |maybe_members| { + translated.saturating_inc(); + maybe_members.map(|members| { + let bounded_members: BoundedVec< + SeatHolder>, + T::DesiredMembers, + > = members.try_into().unwrap(); + bounded_members + }) + }, + ); + T::DbWeight::get().reads_writes(translated, translated) +} + +pub fn migrate_runners_up() -> Weight { + let mut translated = 0u64; + let _ = crate::RunnersUp::::translate::>>, _>( + |maybe_runners_up| { + translated.saturating_inc(); + maybe_runners_up.map(|runners_up| { + let bounded_runners_up: BoundedVec< + SeatHolder>, + T::DesiredRunnersUp, + > = runners_up.try_into().unwrap(); + bounded_runners_up + }) + }, + ); + T::DbWeight::get().reads_writes(translated, translated) +} + +pub fn migrate_candidates() -> Weight { + let mut translated = 0u64; + let _ = crate::Candidates::::translate::)>, _>( + |maybe_candidates| { + translated.saturating_inc(); + maybe_candidates.map(|candidates| { + let bounded_candidates: BoundedVec<(T::AccountId, BalanceOf), T::MaxCandidates> = + candidates.try_into().unwrap(); + bounded_candidates + }) + }, + ); + T::DbWeight::get().reads_writes(translated, translated) +} + +pub fn post_migration() { + log::info!("post-migration elections-phragmen"); + + assert_eq!(StorageVersion::get::>(), 6); } From 68d3cbc08e82dbbd432e5d0c72b0dd7f0e381cd0 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sun, 27 Nov 2022 10:28:13 +0100 Subject: [PATCH 27/34] change copyright year --- frame/elections-phragmen/src/migrations/v6.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/migrations/v6.rs b/frame/elections-phragmen/src/migrations/v6.rs index d13c18ca0eb16..73cb8832c3e4b 100644 --- a/frame/elections-phragmen/src/migrations/v6.rs +++ b/frame/elections-phragmen/src/migrations/v6.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2021-2022 Parity Technologies (UK) Ltd. +// Copyright (C) 2022 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); From 7f93a1234d2e8f4f80bcefb1eaefa89d847dd106 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sun, 27 Nov 2022 10:45:40 +0100 Subject: [PATCH 28/34] error handling --- frame/elections-phragmen/src/lib.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 93a3626ac435b..56a9ac6442cf1 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -795,9 +795,13 @@ impl Pallet { // defensive-only: Members and runners-up are disjoint. This will always be err and // give us an index to insert. if let Err(index) = members.binary_search_by(|m| m.who.cmp(&next_best.who)) { - members - .try_insert(index, next_best.clone()) - .expect("Cannot accept more than DesiredMembers."); + let _ = members.try_insert(index, next_best.clone()).map_err(|_| { + // This can never happen. + log::error!( + target: "runtime::elections-phragmen", + "There must be an empty place for a new member since we just removed one.", + ); + }); } else { // overlap. This can never happen. If so, it seems like our intended replacement // is already a member, so not much more to do. From 21fa0dcf08c8f79c6161a6c247ca5cb471a94330 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sun, 4 Dec 2022 18:53:02 +0100 Subject: [PATCH 29/34] try-runtime --- frame/elections-phragmen/src/migrations/v6.rs | 35 ++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/frame/elections-phragmen/src/migrations/v6.rs b/frame/elections-phragmen/src/migrations/v6.rs index 73cb8832c3e4b..618fb9b668a99 100644 --- a/frame/elections-phragmen/src/migrations/v6.rs +++ b/frame/elections-phragmen/src/migrations/v6.rs @@ -17,7 +17,11 @@ use crate::{BalanceOf, Config, Pallet, SeatHolder, Weight, MAXIMUM_VOTE}; use codec::{Decode, Encode}; -use frame_support::{pallet_prelude::StorageVersion, traits::ConstU32, BoundedVec, RuntimeDebug}; +use frame_support::{ + pallet_prelude::StorageVersion, + traits::{ConstU32, GetStorageVersion, OnRuntimeUpgrade}, + BoundedVec, RuntimeDebug, +}; use sp_core::Get; use sp_runtime::Saturating; use sp_std::prelude::*; @@ -116,8 +120,31 @@ pub fn migrate_candidates() -> Weight { T::DbWeight::get().reads_writes(translated, translated) } -pub fn post_migration() { - log::info!("post-migration elections-phragmen"); +pub struct MigrateToV6(sp_std::marker::PhantomData); +impl OnRuntimeUpgrade for MigrateToV6 { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + let members_count = crate::Members::::get().len(); + Ok((members_count as u32).encode()) + } + + fn on_runtime_upgrade() -> Weight { + apply::() + } - assert_eq!(StorageVersion::get::>(), 6); + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { + let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) + .expect("the state parameter should be something that was generated by pre_upgrade"); + let post_members_count = crate::Members::::get().len() as u32; + assert_eq!( + prev_count.min(T::DesiredMembers::get()), + post_members_count, + "The members count should be the same or less." + ); + + // new version must be set. + assert_eq!(Pallet::::on_chain_storage_version(), 6); + Ok(()) + } } From f2aebebd318643410c404f05ff650e1ac2971a22 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sun, 4 Dec 2022 19:08:46 +0100 Subject: [PATCH 30/34] remove warning --- frame/elections-phragmen/src/migrations/v6.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/elections-phragmen/src/migrations/v6.rs b/frame/elections-phragmen/src/migrations/v6.rs index 618fb9b668a99..92e8bdbd763da 100644 --- a/frame/elections-phragmen/src/migrations/v6.rs +++ b/frame/elections-phragmen/src/migrations/v6.rs @@ -19,13 +19,16 @@ use crate::{BalanceOf, Config, Pallet, SeatHolder, Weight, MAXIMUM_VOTE}; use codec::{Decode, Encode}; use frame_support::{ pallet_prelude::StorageVersion, - traits::{ConstU32, GetStorageVersion, OnRuntimeUpgrade}, + traits::{ConstU32, OnRuntimeUpgrade}, BoundedVec, RuntimeDebug, }; use sp_core::Get; use sp_runtime::Saturating; use sp_std::prelude::*; +#[cfg(feature = "try-runtime")] +use frame_support::traits::GetStorageVersion; + #[derive(Encode, Decode, Clone, Default, RuntimeDebug, PartialEq)] struct DeprecatedVoter { votes: Vec, From fd58b5a3de6b51fc0c3c29af4e141df916ad98cd Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Mon, 5 Dec 2022 22:29:31 +0100 Subject: [PATCH 31/34] testing migrations --- frame/elections-phragmen/src/migrations/v6.rs | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/frame/elections-phragmen/src/migrations/v6.rs b/frame/elections-phragmen/src/migrations/v6.rs index 92e8bdbd763da..97f5daa02e063 100644 --- a/frame/elections-phragmen/src/migrations/v6.rs +++ b/frame/elections-phragmen/src/migrations/v6.rs @@ -128,7 +128,15 @@ impl OnRuntimeUpgrade for MigrateToV6 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { let members_count = crate::Members::::get().len(); - Ok((members_count as u32).encode()) + log::info!("{} members will be migrated.", members_count); + + let runners_up_count = crate::RunnersUp::::get().len(); + log::info!("{} runners-up will be migrated.", runners_up_count); + + let candidates_count = crate::Candidates::::get().len(); + log::info!("{} candidates will be migrated.", candidates_count); + + Ok((members_count as u32, runners_up_count as u32, candidates_count as u32).encode()) } fn on_runtime_upgrade() -> Weight { @@ -136,15 +144,29 @@ impl OnRuntimeUpgrade for MigrateToV6 { } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { - let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) - .expect("the state parameter should be something that was generated by pre_upgrade"); + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + let (old_members_count, old_runners_up_count, old_candidates_count): (u32, u32, u32) = + Decode::decode(&mut &state[..]).expect("pre_upgrade provides a valid state; qed"); + let post_members_count = crate::Members::::get().len() as u32; + let post_runners_up_count = crate::RunnersUp::::get().len() as u32; + let post_candidates_count = crate::Candidates::::get().len() as u32; + assert_eq!( - prev_count.min(T::DesiredMembers::get()), + old_members_count.min(T::DesiredMembers::get()), post_members_count, "The members count should be the same or less." ); + assert_eq!( + old_runners_up_count.min(T::DesiredRunnersUp::get()), + post_runners_up_count, + "The runners-up count should be the same or less." + ); + assert_eq!( + old_candidates_count.min(T::MaxCandidates::get()), + post_candidates_count, + "The runners-up count should be the same or less." + ); // new version must be set. assert_eq!(Pallet::::on_chain_storage_version(), 6); From 1690a0076c6996bf5205b3b757a4140c463bd0e3 Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Mon, 27 Feb 2023 17:05:24 +0100 Subject: [PATCH 32/34] pallet & migration builds; tests don't --- frame/elections-phragmen/src/lib.rs | 31 ++++++++++++------- frame/elections-phragmen/src/migrations/v6.rs | 8 ++--- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 296f830287e5e..d8932426c4bb5 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -101,7 +101,7 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ traits::{ - defensive_prelude::*, ChangeMembers, ConstU32, Contains, ContainsLengthBound, Currency, + defensive_prelude::*, ChangeMembers, Contains, ContainsLengthBound, Currency, CurrencyToVote, Get, InitializeMembers, LockIdentifier, LockableCurrency, OnUnbalanced, ReservableCurrency, SortedMembers, WithdrawReasons, }, @@ -144,9 +144,10 @@ pub enum Renouncing { /// An active voter. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, TypeInfo, MaxEncodedLen)] -pub struct Voter { +#[scale_info(skip_type_params(MaxVotesPerVoter))] +pub struct Voter> { /// The members being backed. - pub votes: BoundedVec>, + pub votes: BoundedVec, /// The amount of stake placed on this vote. pub stake: Balance, /// The amount of deposit reserved for this vote. @@ -155,7 +156,9 @@ pub struct Voter { pub deposit: Balance, } -impl Default for Voter { +impl> Default + for Voter +{ fn default() -> Self { Self { votes: BoundedVec::default(), @@ -361,15 +364,14 @@ pub mod pallet { )] pub fn vote( origin: OriginFor, - votes: BoundedVec>, + votes: Vec, #[pallet::compact] value: BalanceOf, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - ensure!( - votes.len() <= T::MaxVotesPerVoter::get() as usize, - Error::::MaximumVotesExceeded - ); + let votes: BoundedVec = + votes.try_into().map_err(|_| Error::::MaximumVotesExceeded)?; + ensure!(!votes.is_empty(), Error::::NoVotes); let candidates_count = >::decode_len().unwrap_or(0); @@ -713,8 +715,13 @@ pub mod pallet { /// TWOX-NOTE: SAFE as `AccountId` is a crypto hash. #[pallet::storage] #[pallet::getter(fn voting)] - pub type Voting = - StorageMap<_, Twox64Concat, T::AccountId, Voter>, ValueQuery>; + pub type Voting = StorageMap< + _, + Twox64Concat, + T::AccountId, + Voter, T::MaxVotesPerVoter>, + ValueQuery, + >; #[pallet::genesis_config] pub struct GenesisConfig { @@ -1575,7 +1582,7 @@ mod tests { Elections::vote(origin, votes.try_into().unwrap(), stake) } - fn votes_of(who: &u64) -> BoundedVec> { + fn votes_of(who: &u64) -> BoundedVec::MaxVotesPerVoter> { Voting::::get(who).votes } diff --git a/frame/elections-phragmen/src/migrations/v6.rs b/frame/elections-phragmen/src/migrations/v6.rs index 97f5daa02e063..6e0a06da2bada 100644 --- a/frame/elections-phragmen/src/migrations/v6.rs +++ b/frame/elections-phragmen/src/migrations/v6.rs @@ -15,12 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{BalanceOf, Config, Pallet, SeatHolder, Weight, MAXIMUM_VOTE}; +use crate::{BalanceOf, Config, Pallet, SeatHolder, Weight}; use codec::{Decode, Encode}; use frame_support::{ - pallet_prelude::StorageVersion, - traits::{ConstU32, OnRuntimeUpgrade}, - BoundedVec, RuntimeDebug, + pallet_prelude::StorageVersion, traits::OnRuntimeUpgrade, BoundedVec, RuntimeDebug, }; use sp_core::Get; use sp_runtime::Saturating; @@ -67,7 +65,7 @@ pub fn migrate_voting() -> Weight { let mut translated = 0u64; crate::Voting::::translate::>, _>(|_, voter| { translated.saturating_inc(); - let bounded_votes: BoundedVec> = + let bounded_votes: BoundedVec = voter.votes.try_into().unwrap(); Some(crate::Voter { votes: bounded_votes, stake: voter.stake, deposit: voter.deposit }) }); From e13bb41653c37833e0090b334c2a2e7d6cd2653c Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Mon, 27 Feb 2023 17:07:11 +0100 Subject: [PATCH 33/34] update extrinsic votes arg --- frame/elections-phragmen/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index d8932426c4bb5..d8e75f45fa66e 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -364,14 +364,11 @@ pub mod pallet { )] pub fn vote( origin: OriginFor, - votes: Vec, + votes: BoundedVec, #[pallet::compact] value: BalanceOf, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - let votes: BoundedVec = - votes.try_into().map_err(|_| Error::::MaximumVotesExceeded)?; - ensure!(!votes.is_empty(), Error::::NoVotes); let candidates_count = >::decode_len().unwrap_or(0); From 839f85dbb8c2a884b258d459db709ca4f0082cdf Mon Sep 17 00:00:00 2001 From: Sergej Sakac Date: Sun, 5 Mar 2023 17:54:26 +0100 Subject: [PATCH 34/34] safer approach --- frame/elections-phragmen/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index d8e75f45fa66e..be6ff82af6fce 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -1156,6 +1156,7 @@ impl Pallet { who: who.clone(), stake: *stake, }) + .take(T::DesiredMembers::get() as usize) .collect::>() .try_into() .expect("number members will never exceed T::DesiredMembers. qed."), @@ -1169,6 +1170,7 @@ impl Pallet { who, stake, }) + .take(T::DesiredRunnersUp::get() as usize) .collect::>() .try_into() .expect(