difftreelog
feat(collator-selection) method refactoring + unit tests complete
in: master
9 files changed
pallets/collator-selection/src/benchmarking.rsdiffbeforeafterboth1// Copyright 2019-2022 Unique Network (Gibraltar) Ltd.2// This file is part of Unique Network.34// Unique Network is free software: you can redistribute it and/or modify5// it under the terms of the GNU General Public License as published by6// the Free Software Foundation, either version 3 of the License, or7// (at your option) any later version.89// Unique Network is distributed in the hope that it will be useful,10// but WITHOUT ANY WARRANTY; without even the implied warranty of11// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the12// GNU General Public License for more details.1314// You should have received a copy of the GNU General Public License15// along with Unique Network. If not, see <http://www.gnu.org/licenses/>.1617// Original license:18// Copyright (C) 2021 Parity Technologies (UK) Ltd.19// SPDX-License-Identifier: Apache-2.02021// Licensed under the Apache License, Version 2.0 (the "License");22// you may not use this file except in compliance with the License.23// You may obtain a copy of the License at24//25// http://www.apache.org/licenses/LICENSE-2.026//27// Unless required by applicable law or agreed to in writing, software28// distributed under the License is distributed on an "AS IS" BASIS,29// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.30// See the License for the specific language governing permissions and31// limitations under the License.3233//! Benchmarking setup for pallet-collator-selection3435use super::*;3637#[allow(unused)]38use crate::Pallet as CollatorSelection;39use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};40use frame_support::{41 assert_ok,42 codec::Decode,43 traits::{Currency, EnsureOrigin, Get},44};45use frame_system::{EventRecord, RawOrigin};46use pallet_authorship::EventHandler;47use pallet_session::{self as session, SessionManager};48use sp_std::prelude::*;4950pub type BalanceOf<T> =51 <<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;5253const SEED: u32 = 0;5455// TODO: remove if this is given in substrate commit.56macro_rules! whitelist {57 ($acc:ident) => {58 frame_benchmarking::benchmarking::add_to_whitelist(59 frame_system::Account::<T>::hashed_key_for(&$acc).into(),60 );61 };62}6364fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {65 let events = frame_system::Pallet::<T>::events();66 let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();67 // compare to the last event record68 let EventRecord { event, .. } = &events[events.len() - 1];69 assert_eq!(event, &system_event);70}7172fn create_funded_user<T: Config>(73 string: &'static str,74 n: u32,75 balance_factor: u32,76) -> T::AccountId {77 let user = account(string, n, SEED);78 let balance = T::Currency::minimum_balance() * balance_factor.into();79 let _ = T::Currency::make_free_balance_be(&user, balance);80 user81}8283fn keys<T: Config + session::Config>(c: u32) -> <T as session::Config>::Keys {84 use rand::{RngCore, SeedableRng};8586 let keys = {87 let mut keys = [0u8; 128];8889 if c > 0 {90 let mut rng = rand::rngs::StdRng::seed_from_u64(c as u64);91 rng.fill_bytes(&mut keys);92 }9394 keys95 };9697 Decode::decode(&mut &keys[..]).unwrap()98}99100fn validator<T: Config + session::Config>(c: u32) -> (T::AccountId, <T as session::Config>::Keys) {101 (create_funded_user::<T>("candidate", c, 1000), keys::<T>(c))102}103104fn register_validators<T: Config + session::Config>(count: u32) -> Vec<T::AccountId> {105 let validators = (0..count).map(|c| validator::<T>(c)).collect::<Vec<_>>();106107 for (who, keys) in validators.clone() {108 <session::Pallet<T>>::set_keys(RawOrigin::Signed(who).into(), keys, Vec::new()).unwrap();109 }110111 validators.into_iter().map(|(who, _)| who).collect()112}113114fn register_candidates<T: Config>(count: u32) {115 let candidates = (0..count)116 .map(|c| account("candidate", c, SEED))117 .collect::<Vec<_>>();118 assert!(119 <LicenseBond<T>>::get() > 0u32.into(),120 "Bond cannot be zero!"121 );122123 for who in candidates {124 T::Currency::make_free_balance_be(&who, <LicenseBond<T>>::get() * 2u32.into());125 <CollatorSelection<T>>::register_as_candidate(RawOrigin::Signed(who).into()).unwrap();126 }127}128129benchmarks! {130 where_clause { where T: pallet_authorship::Config + session::Config }131132 set_invulnerables {133 let b in 1 .. T::MaxInvulnerables::get();134 let new_invulnerables = register_validators::<T>(b);135 let origin = T::UpdateOrigin::successful_origin();136 }: {137 assert_ok!(138 <CollatorSelection<T>>::set_invulnerables(origin, new_invulnerables.clone())139 );140 }141 verify {142 assert_last_event::<T>(Event::NewInvulnerables{invulnerables: new_invulnerables}.into());143 }144145 set_desired_candidates {146 let max: u32 = 999;147 let origin = T::UpdateOrigin::successful_origin();148 }: {149 assert_ok!(150 <CollatorSelection<T>>::set_desired_candidates(origin, max.clone())151 );152 }153 verify {154 assert_last_event::<T>(Event::NewDesiredCandidates{desired_candidates: max}.into());155 }156157 set_license_bond {158 let bond_amount: BalanceOf<T> = T::Currency::minimum_balance() * 10u32.into();159 let origin = T::UpdateOrigin::successful_origin();160 }: {161 assert_ok!(162 <CollatorSelection<T>>::set_license_bond(origin, bond_amount.clone())163 );164 }165 verify {166 assert_last_event::<T>(Event::NewLicenseBond{bond_amount}.into());167 }168169 // worse case is when we have all the max-candidate slots filled except one, and we fill that170 // one.171 register_as_candidate {172 let c in 1 .. T::MaxCandidates::get();173174 <LicenseBond<T>>::put(T::Currency::minimum_balance());175 <DesiredCandidates<T>>::put(c + 1);176177 register_validators::<T>(c);178 register_candidates::<T>(c);179180 let caller: T::AccountId = whitelisted_caller();181 let bond: BalanceOf<T> = T::Currency::minimum_balance() * 2u32.into();182 T::Currency::make_free_balance_be(&caller, bond.clone());183184 <session::Pallet<T>>::set_keys(185 RawOrigin::Signed(caller.clone()).into(),186 keys::<T>(c + 1),187 Vec::new()188 ).unwrap();189190 }: _(RawOrigin::Signed(caller.clone()))191 verify {192 assert_last_event::<T>(Event::CandidateAdded{account_id: caller, deposit: bond / 2u32.into()}.into());193 }194195 // worse case is the last candidate leaving.196 leave_intent {197 let c in (T::MinCandidates::get() + 1) .. T::MaxCandidates::get();198 <LicenseBond<T>>::put(T::Currency::minimum_balance());199 <DesiredCandidates<T>>::put(c);200201 register_validators::<T>(c);202 register_candidates::<T>(c);203204 let leaving = <Candidates<T>>::get().last().unwrap().who.clone();205 whitelist!(leaving);206 }: _(RawOrigin::Signed(leaving.clone()))207 verify {208 // todo:collator verify these209 assert_last_event::<T>(Event::CandidateRemoved{account_id: leaving, deposit_returned: bond / 2u32.into() }.into());210 }211212 // worse case is paying a non-existing candidate account.213 note_author {214 <LicenseBond<T>>::put(T::Currency::minimum_balance());215 T::Currency::make_free_balance_be(216 &<CollatorSelection<T>>::account_id(),217 T::Currency::minimum_balance() * 4u32.into(),218 );219 let author = account("author", 0, SEED);220 let new_block: T::BlockNumber = 10u32.into();221222 frame_system::Pallet::<T>::set_block_number(new_block);223 assert!(T::Currency::free_balance(&author) == 0u32.into());224 }: {225 <CollatorSelection<T> as EventHandler<_, _>>::note_author(author.clone())226 } verify {227 assert!(T::Currency::free_balance(&author) > 0u32.into());228 assert_eq!(frame_system::Pallet::<T>::block_number(), new_block);229 }230231 // worst case for new session.232 new_session {233 let r in 1 .. T::MaxCandidates::get();234 let c in 1 .. T::MaxCandidates::get();235236 <LicenseBond<T>>::put(T::Currency::minimum_balance());237 <DesiredCandidates<T>>::put(c);238 frame_system::Pallet::<T>::set_block_number(0u32.into());239240 register_validators::<T>(c);241 register_candidates::<T>(c);242243 let new_block: T::BlockNumber = 1800u32.into();244 let zero_block: T::BlockNumber = 0u32.into();245 let candidates = <Candidates<T>>::get();246247 let non_removals = c.saturating_sub(r);248249 for i in 0..c {250 <LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), zero_block);251 }252253 if non_removals > 0 {254 for i in 0..non_removals {255 <LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), new_block);256 }257 } else {258 for i in 0..c {259 <LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), new_block);260 }261 }262263 let pre_length = <Candidates<T>>::get().len();264265 frame_system::Pallet::<T>::set_block_number(new_block);266267 assert!(<Candidates<T>>::get().len() == c as usize);268 }: {269 <CollatorSelection<T> as SessionManager<_>>::new_session(0)270 } verify {271 if c > r && non_removals >= T::MinCandidates::get() {272 assert!(<Candidates<T>>::get().len() < pre_length);273 } else if c > r && non_removals < T::MinCandidates::get() {274 assert!(<Candidates<T>>::get().len() == T::MinCandidates::get() as usize);275 } else {276 assert!(<Candidates<T>>::get().len() == pre_length);277 }278 }279}280281impl_benchmark_test_suite!(282 CollatorSelection,283 crate::mock::new_test_ext(),284 crate::mock::Test,285);1// Copyright 2019-2022 Unique Network (Gibraltar) Ltd.2// This file is part of Unique Network.34// Unique Network is free software: you can redistribute it and/or modify5// it under the terms of the GNU General Public License as published by6// the Free Software Foundation, either version 3 of the License, or7// (at your option) any later version.89// Unique Network is distributed in the hope that it will be useful,10// but WITHOUT ANY WARRANTY; without even the implied warranty of11// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the12// GNU General Public License for more details.1314// You should have received a copy of the GNU General Public License15// along with Unique Network. If not, see <http://www.gnu.org/licenses/>.1617// Original license:18// Copyright (C) 2021 Parity Technologies (UK) Ltd.19// SPDX-License-Identifier: Apache-2.02021// Licensed under the Apache License, Version 2.0 (the "License");22// you may not use this file except in compliance with the License.23// You may obtain a copy of the License at24//25// http://www.apache.org/licenses/LICENSE-2.026//27// Unless required by applicable law or agreed to in writing, software28// distributed under the License is distributed on an "AS IS" BASIS,29// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.30// See the License for the specific language governing permissions and31// limitations under the License.3233//! Benchmarking setup for pallet-collator-selection3435use super::*;3637#[allow(unused)]38use crate::Pallet as CollatorSelection;39use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller};40use frame_support::{41 assert_ok,42 codec::Decode,43 traits::{Currency, EnsureOrigin, Get},44};45use frame_system::{EventRecord, RawOrigin};46use pallet_authorship::EventHandler;47use pallet_session::{self as session, SessionManager};48use sp_std::prelude::*;4950pub type BalanceOf<T> =51 <<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;5253const SEED: u32 = 0;5455// TODO: remove if this is given in substrate commit.56macro_rules! whitelist {57 ($acc:ident) => {58 frame_benchmarking::benchmarking::add_to_whitelist(59 frame_system::Account::<T>::hashed_key_for(&$acc).into(),60 );61 };62}6364fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {65 let events = frame_system::Pallet::<T>::events();66 let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();67 // compare to the last event record68 let EventRecord { event, .. } = &events[events.len() - 1];69 assert_eq!(event, &system_event);70}7172fn create_funded_user<T: Config>(73 string: &'static str,74 n: u32,75 balance_factor: u32,76) -> T::AccountId {77 let user = account(string, n, SEED);78 let balance = T::Currency::minimum_balance() * balance_factor.into();79 let _ = T::Currency::make_free_balance_be(&user, balance);80 user81}8283fn keys<T: Config + session::Config>(c: u32) -> <T as session::Config>::Keys {84 use rand::{RngCore, SeedableRng};8586 let keys = {87 let mut keys = [0u8; 128];8889 if c > 0 {90 let mut rng = rand::rngs::StdRng::seed_from_u64(c as u64);91 rng.fill_bytes(&mut keys);92 }9394 keys95 };9697 Decode::decode(&mut &keys[..]).unwrap()98}99100fn validator<T: Config + session::Config>(c: u32) -> (T::AccountId, <T as session::Config>::Keys) {101 (create_funded_user::<T>("candidate", c, 1000), keys::<T>(c))102}103104fn register_validators<T: Config + session::Config>(count: u32) -> Vec<T::AccountId> {105 let validators = (0..count).map(|c| validator::<T>(c)).collect::<Vec<_>>();106107 for (who, keys) in validators.clone() {108 <session::Pallet<T>>::set_keys(RawOrigin::Signed(who).into(), keys, Vec::new()).unwrap();109 }110111 validators.into_iter().map(|(who, _)| who).collect()112}113114fn register_candidates<T: Config>(count: u32) {115 let candidates = (0..count)116 .map(|c| account("candidate", c, SEED))117 .collect::<Vec<_>>();118 assert!(119 <LicenseBond<T>>::get() > 0u32.into(),120 "Bond cannot be zero!"121 );122123 for who in candidates {124 T::Currency::make_free_balance_be(&who, <LicenseBond<T>>::get() * 2u32.into());125 <CollatorSelection<T>>::register_as_candidate(RawOrigin::Signed(who).into()).unwrap();126 }127}128129benchmarks! {130 where_clause { where T: pallet_authorship::Config + session::Config }131132 set_invulnerables {133 let b in 1 .. T::MaxCollators::get();134 let new_invulnerables = register_validators::<T>(b);135 let origin = T::UpdateOrigin::successful_origin();136 }: {137 assert_ok!(138 <CollatorSelection<T>>::set_invulnerables(origin, new_invulnerables.clone())139 );140 }141 verify {142 assert_last_event::<T>(Event::NewInvulnerables{invulnerables: new_invulnerables}.into());143 }144145 set_desired_collators {146 let max: u32 = 999;147 let origin = T::UpdateOrigin::successful_origin();148 }: {149 assert_ok!(150 <CollatorSelection<T>>::set_desired_collators(origin, max.clone())151 );152 }153 verify {154 assert_last_event::<T>(Event::NewDesiredCollators{desired_collators: max}.into());155 }156157 set_license_bond {158 let bond_amount: BalanceOf<T> = T::Currency::minimum_balance() * 10u32.into();159 let origin = T::UpdateOrigin::successful_origin();160 }: {161 assert_ok!(162 <CollatorSelection<T>>::set_license_bond(origin, bond_amount.clone())163 );164 }165 verify {166 assert_last_event::<T>(Event::NewLicenseBond{bond_amount}.into());167 }168169 // worse case is when we have all the max-candidate slots filled except one, and we fill that170 // one.171 register_as_candidate {172 let c in 1 .. T::MaxCollators::get();173174 <LicenseBond<T>>::put(T::Currency::minimum_balance());175 <DesiredCollators<T>>::put(c + 1);176177 register_validators::<T>(c);178 register_candidates::<T>(c);179180 let caller: T::AccountId = whitelisted_caller();181 let bond: BalanceOf<T> = T::Currency::minimum_balance() * 2u32.into();182 T::Currency::make_free_balance_be(&caller, bond.clone());183184 <session::Pallet<T>>::set_keys(185 RawOrigin::Signed(caller.clone()).into(),186 keys::<T>(c + 1),187 Vec::new()188 ).unwrap();189190 }: _(RawOrigin::Signed(caller.clone()))191 verify {192 assert_last_event::<T>(Event::CandidateAdded{account_id: caller, deposit: bond / 2u32.into()}.into());193 }194195 // worse case is the last candidate leaving.196 leave_intent {197 let c in (T::MinCandidates::get() + 1) .. T::MaxCollators::get();198 <LicenseBond<T>>::put(T::Currency::minimum_balance());199 <DesiredCollators<T>>::put(c);200201 register_validators::<T>(c);202 register_candidates::<T>(c);203204 let leaving = <Candidates<T>>::get().last().unwrap().who.clone();205 whitelist!(leaving);206 }: _(RawOrigin::Signed(leaving.clone()))207 verify {208 // todo:collator verify these209 assert_last_event::<T>(Event::CandidateRemoved{account_id: leaving, deposit_returned: bond / 2u32.into() }.into());210 }211212 // worse case is paying a non-existing candidate account.213 note_author {214 <LicenseBond<T>>::put(T::Currency::minimum_balance());215 T::Currency::make_free_balance_be(216 &<CollatorSelection<T>>::account_id(),217 T::Currency::minimum_balance() * 4u32.into(),218 );219 let author = account("author", 0, SEED);220 let new_block: T::BlockNumber = 10u32.into();221222 frame_system::Pallet::<T>::set_block_number(new_block);223 assert!(T::Currency::free_balance(&author) == 0u32.into());224 }: {225 <CollatorSelection<T> as EventHandler<_, _>>::note_author(author.clone())226 } verify {227 assert!(T::Currency::free_balance(&author) > 0u32.into());228 assert_eq!(frame_system::Pallet::<T>::block_number(), new_block);229 }230231 // worst case for new session.232 new_session {233 let r in 1 .. T::MaxCollators::get();234 let c in 1 .. T::MaxCollators::get();235236 <LicenseBond<T>>::put(T::Currency::minimum_balance());237 <DesiredCollators<T>>::put(c);238 frame_system::Pallet::<T>::set_block_number(0u32.into());239240 register_validators::<T>(c);241 register_candidates::<T>(c);242243 let new_block: T::BlockNumber = 1800u32.into();244 let zero_block: T::BlockNumber = 0u32.into();245 let candidates = <Candidates<T>>::get();246247 let non_removals = c.saturating_sub(r);248249 for i in 0..c {250 <LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), zero_block);251 }252253 if non_removals > 0 {254 for i in 0..non_removals {255 <LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), new_block);256 }257 } else {258 for i in 0..c {259 <LastAuthoredBlock<T>>::insert(candidates[i as usize].who.clone(), new_block);260 }261 }262263 let pre_length = <Candidates<T>>::get().len();264265 frame_system::Pallet::<T>::set_block_number(new_block);266267 assert!(<Candidates<T>>::get().len() == c as usize);268 }: {269 <CollatorSelection<T> as SessionManager<_>>::new_session(0)270 } verify {271 if c > r && non_removals >= T::MinCandidates::get() {272 assert!(<Candidates<T>>::get().len() < pre_length);273 } else if c > r && non_removals < T::MinCandidates::get() {274 assert!(<Candidates<T>>::get().len() == T::MinCandidates::get() as usize);275 } else {276 assert!(<Candidates<T>>::get().len() == pre_length);277 }278 }279}280281impl_benchmark_test_suite!(282 CollatorSelection,283 crate::mock::new_test_ext(),284 crate::mock::Test,285);pallets/collator-selection/src/lib.rsdiffbeforeafterboth--- a/pallets/collator-selection/src/lib.rs
+++ b/pallets/collator-selection/src/lib.rs
@@ -98,10 +98,7 @@
dispatch::{DispatchClass, DispatchResultWithPostInfo},
inherent::Vec,
pallet_prelude::*,
- sp_runtime::{
- traits::{AccountIdConversion, CheckedSub, Saturating, Zero},
- RuntimeDebug,
- },
+ sp_runtime::traits::{AccountIdConversion, CheckedSub, Saturating, Zero},
traits::{
Currency, EnsureOrigin, ExistenceRequirement::KeepAlive, ReservableCurrency,
ValidatorRegistration,
@@ -145,19 +142,9 @@
/// Account Identifier from which the internal Pot is generated.
type PotId: Get<PalletId>;
-
- /// Maximum number of candidates that we should have. This is enforced in code.
- ///
- /// This does not take into account the invulnerables.
- type MaxCandidates: Get<u32>;
- /// Minimum number of candidates that we should have. This is used for disaster recovery.
- ///
- /// This does not take into account the invulnerables.
- type MinCandidates: Get<u32>;
-
- /// Maximum number of invulnerables. This is enforced in code.
- type MaxInvulnerables: Get<u32>;
+ /// Maximum number of candidates and invulnerables that we should have. This is enforced in code.
+ type MaxCollators: Get<u32>;
/// If kicked, how much of the collator's deposit will be slashed and sent to the slash destination.
type SlashRatio: Get<Perbill>;
@@ -175,17 +162,6 @@
/// The weight information of this pallet.
type WeightInfo: WeightInfo;
- }
-
- /// Basic information about a collation candidate.
- #[derive(
- PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, scale_info::TypeInfo, MaxEncodedLen,
- )]
- pub struct LicenseInfo<AccountId, Balance> {
- /// Account identifier.
- pub who: AccountId,
- /// Reserved deposit.
- pub deposit: Balance,
}
#[pallet::pallet]
@@ -196,7 +172,7 @@
#[pallet::storage]
#[pallet::getter(fn invulnerables)]
pub type Invulnerables<T: Config> =
- StorageValue<_, BoundedVec<T::AccountId, T::MaxInvulnerables>, ValueQuery>;
+ StorageValue<_, BoundedVec<T::AccountId, T::MaxCollators>, ValueQuery>;
/// The (community) collation license holders.
#[pallet::storage]
@@ -209,7 +185,7 @@
#[pallet::getter(fn candidates)]
pub type Candidates<T: Config> = StorageValue<
_,
- BoundedVec<T::AccountId, T::MaxCandidates>, //LicenseInfo<T::AccountId, BalanceOf<T>>, T::MaxCandidates>, // license ID?
+ BoundedVec<T::AccountId, T::MaxCollators>, //LicenseInfo<T::AccountId, BalanceOf<T>>, T::MaxCollators>, // license ID?
ValueQuery,
>;
@@ -228,10 +204,10 @@
/// Desired number of candidates.
///
- /// This should ideally always be less than [`Config::MaxCandidates`] for weights to be correct.
+ /// This should ideally always be less than [`Config::MaxCollators`] for weights to be correct.
#[pallet::storage]
- #[pallet::getter(fn desired_candidates)]
- pub type DesiredCandidates<T> = StorageValue<_, u32, ValueQuery>;
+ #[pallet::getter(fn desired_collators)]
+ pub type DesiredCollators<T> = StorageValue<_, u32, ValueQuery>;
/// Fixed amount to deposit to become a collator.
///
@@ -245,7 +221,7 @@
pub invulnerables: Vec<T::AccountId>,
pub license_bond: BalanceOf<T>,
pub kick_threshold: T::BlockNumber,
- pub desired_candidates: u32,
+ pub desired_collators: u32,
}
#[cfg(feature = "std")]
@@ -255,7 +231,7 @@
invulnerables: Default::default(),
license_bond: Default::default(),
kick_threshold: T::BlockNumber::one(),
- desired_candidates: Default::default(),
+ desired_collators: Default::default(),
}
}
}
@@ -273,16 +249,16 @@
);
let bounded_invulnerables =
- BoundedVec::<_, T::MaxInvulnerables>::try_from(self.invulnerables.clone())
- .expect("genesis invulnerables are more than T::MaxInvulnerables");
+ BoundedVec::<_, T::MaxCollators>::try_from(self.invulnerables.clone())
+ .expect("genesis invulnerables are more than T::MaxCollators");
assert!(
- T::MaxCandidates::get() >= self.desired_candidates,
- "genesis desired_candidates are more than T::MaxCandidates",
+ T::MaxCollators::get() >= self.desired_collators,
+ "genesis desired_collators are more than T::MaxCollators",
);
- <DesiredCandidates<T>>::put(&self.desired_candidates);
- <LicenseBond<T>>::put(&self.license_bond);
- <KickThreshold<T>>::put(&self.kick_threshold);
+ <DesiredCollators<T>>::put(self.desired_collators);
+ <LicenseBond<T>>::put(self.license_bond);
+ <KickThreshold<T>>::put(self.kick_threshold);
<Invulnerables<T>>::put(bounded_invulnerables);
}
}
@@ -290,8 +266,8 @@
#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
- NewDesiredCandidates {
- desired_candidates: u32,
+ NewDesiredCollators {
+ desired_collators: u32,
},
NewLicenseBond {
bond_amount: BalanceOf<T>,
@@ -326,14 +302,12 @@
pub enum Error<T> {
/// Too many candidates
TooManyCandidates,
- /// Too few candidates
- TooFewCandidates,
/// Unknown error
Unknown,
/// Permission issue
Permission,
/// User already holds license to collate
- AlreadyLicenseHolder,
+ AlreadyHoldingLicense,
/// User does not hold a license to collate
NoLicense,
/// User is already a candidate
@@ -360,7 +334,7 @@
#[pallet::call]
impl<T: Config> Pallet<T> {
/// Add a collator to the list of invulnerable (fixed) collators.
- #[pallet::weight(T::WeightInfo::set_invulnerables(1 as u32))] // todo:collator weight
+ #[pallet::weight(T::WeightInfo::set_invulnerables(1u32))] // todo:collator weight
pub fn add_invulnerable(
origin: OriginFor<T>,
new: T::AccountId,
@@ -378,12 +352,13 @@
if Self::invulnerables().contains(&new) {
return Ok(().into());
}
-
- // todo:collator check license holders, release moneys, promotion!
- // force_release_license? Error::<T>::lreadyLicenseHolder?
<Invulnerables<T>>::try_append(new.clone())
.map_err(|_| Error::<T>::TooManyInvulnerables)?;
+
+ // try to offboard the new invulnerable if it was a collator candidate before
+ let _ = Self::try_remove_candidate(&new);
+
Self::deposit_event(Event::InvulnerableAdded { invulnerable: new });
Ok(().into())
}
@@ -417,22 +392,19 @@
Ok(().into())
}
- /// Set the ideal number of collators (not including the invulnerables).
- /// If lowering this number, then the number of running collators could be higher than this figure.
+ /// Set the ideal number of collators. If lowering this number,
+ /// then the number of running collators could be higher than this figure.
/// Aside from that edge case, there should be no other way to have more collators than the desired number.
- #[pallet::weight(T::WeightInfo::set_desired_candidates())]
- pub fn set_desired_candidates(
- origin: OriginFor<T>,
- max: u32,
- ) -> DispatchResultWithPostInfo {
+ #[pallet::weight(T::WeightInfo::set_desired_collators())]
+ pub fn set_desired_collators(origin: OriginFor<T>, max: u32) -> DispatchResultWithPostInfo {
T::UpdateOrigin::ensure_origin(origin)?;
// we trust origin calls, this is just a for more accurate benchmarking
- if max > T::MaxCandidates::get() {
- log::warn!("max > T::MaxCandidates; you might need to run benchmarks again");
+ if max > T::MaxCollators::get() {
+ log::warn!("max > T::MaxCollators; you might need to run benchmarks again");
}
- <DesiredCandidates<T>>::put(&max);
- Self::deposit_event(Event::NewDesiredCandidates {
- desired_candidates: max,
+ <DesiredCollators<T>>::put(max);
+ Self::deposit_event(Event::NewDesiredCollators {
+ desired_collators: max,
});
Ok(().into())
}
@@ -444,7 +416,7 @@
bond: BalanceOf<T>,
) -> DispatchResultWithPostInfo {
T::UpdateOrigin::ensure_origin(origin)?;
- <LicenseBond<T>>::put(&bond);
+ <LicenseBond<T>>::put(bond);
Self::deposit_event(Event::NewLicenseBond { bond_amount: bond });
Ok(().into())
}
@@ -470,19 +442,19 @@
/// (a) already have registered session keys and (b) be able to reserve the `LicenseBond`.
///
/// This call is not available to `Invulnerable` collators.
- #[pallet::weight(T::WeightInfo::register_as_candidate(T::MaxCandidates::get()))] // todo:collator weight
+ #[pallet::weight(T::WeightInfo::register_as_candidate(T::MaxCollators::get()))] // todo:collator weight
pub fn get_license(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
// register_as_candidate
let who = ensure_signed(origin)?;
if Licenses::<T>::contains_key(&who) {
- return Ok(().into());
+ return Err(Error::<T>::AlreadyHoldingLicense.into());
}
- ensure!(
+ /*ensure!(
!Self::invulnerables().contains(&who),
Error::<T>::AlreadyInvulnerable
- );
+ );*/
let validator_key = T::ValidatorIdOf::convert(who.clone())
.ok_or(Error::<T>::NoAssociatedValidatorId)?;
@@ -507,7 +479,7 @@
return Err(BadOrigin.into());
}
if candidates.iter().any(|candidate| *candidate == who) {
- Err(Error::<T>::AlreadyLicenseHolder)?
+ Err(Error::<T>::AlreadyHoldingLicense)?
} else {
T::Currency::reserve(&who, deposit)?;
candidates
@@ -532,7 +504,7 @@
/// The account must already hold a license, and cannot offboard immediately during a session.
///
/// This call is not available to `Invulnerable` collators.
- #[pallet::weight(T::WeightInfo::register_as_candidate(T::MaxCandidates::get()))] // todo:collator weight
+ #[pallet::weight(T::WeightInfo::register_as_candidate(T::MaxCollators::get()))] // todo:collator weight
pub fn onboard(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
// register_as_candidate
let who = ensure_signed(origin)?;
@@ -540,9 +512,10 @@
// ensure the user obtained the license.
ensure!(Licenses::<T>::contains_key(&who), Error::<T>::NoLicense);
// ensure we are below limit.
- let length = <Candidates<T>>::decode_len().unwrap_or_default();
+ let length = <Candidates<T>>::decode_len().unwrap_or_default()
+ + <Invulnerables<T>>::decode_len().unwrap_or_default();
ensure!(
- (length as u32) < Self::desired_candidates(),
+ (length as u32) < Self::desired_collators(),
Error::<T>::TooManyCandidates
);
// todo:collator really need it?
@@ -551,8 +524,6 @@
Error::<T>::AlreadyInvulnerable
);
- let deposit = Self::license_bond();
- // First authored block is current block plus kick threshold to handle session delay
/*let incoming = LicenseInfo {
who: who.clone(),
deposit,
@@ -563,10 +534,10 @@
if candidates.iter().any(|candidate| *candidate == who) {
Err(Error::<T>::AlreadyCandidate)?
} else {
- T::Currency::reserve(&who, deposit)?;
candidates
.try_push(who.clone())
.map_err(|_| Error::<T>::TooManyCandidates)?;
+ // First authored block is current block plus kick threshold to handle session delay
<LastAuthoredBlock<T>>::insert(
who.clone(),
frame_system::Pallet::<T>::block_number() + Self::kick_threshold(),
@@ -583,31 +554,31 @@
/// session change. The license to `onboard` later at any other time will remain.
///
/// This call will fail if the total number of candidates would drop below `MinCandidates`. todo:collator maybe not
- #[pallet::weight(T::WeightInfo::leave_intent(T::MaxCandidates::get()))] // todo:collator weight
+ #[pallet::weight(T::WeightInfo::leave_intent(T::MaxCollators::get()))] // todo:collator weight
pub fn offboard(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
// leave_intent
let who = ensure_signed(origin)?;
- // todo:collator invulnerables and candidates should count against min candidates together
+ /* todo:collator invulnerables and candidates should count against min candidates together
ensure!(
Self::candidates().len() as u32 > T::MinCandidates::get(),
Error::<T>::TooFewCandidates
- );
+ );*/
let current_count = Self::try_remove_candidate(&who)?;
- Ok(Some(T::WeightInfo::leave_intent(current_count as u32)).into())
+ Ok(Some(T::WeightInfo::leave_intent(current_count as u32)).into()) // todo:collator weight
}
/// Forfeit `origin`'s own license. The `LicenseBond` will be unreserved immediately.
///
/// This call is not available to `Invulnerable` collators.
- #[pallet::weight(T::WeightInfo::leave_intent(T::MaxCandidates::get()))] // todo:collator weight
+ #[pallet::weight(T::WeightInfo::leave_intent(T::MaxCollators::get()))] // todo:collator weight
pub fn release_license(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
// leave_intent
let who = ensure_signed(origin)?;
- // let current_count = Self::try_remove_candidate(&who, false)?;
- Self::try_release_license(&who, false)?;
- Ok(().into())
+ let current_count = Self::try_remove_candidate_and_release_license(&who, false, true)?;
+
+ Ok(Some(T::WeightInfo::leave_intent(current_count as u32)).into()) // todo:collator weight
}
/// Force deregister `origin` as a collator candidate as a governing authority, and revoke its license.
@@ -615,16 +586,15 @@
/// The `LicenseBond` will be unreserved and returned immediately.
///
/// This call is not available to `Invulnerable` collators.
- #[pallet::weight(T::WeightInfo::leave_intent(T::MaxCandidates::get()))] // todo:collator weight
- pub fn force_release_license(
+ #[pallet::weight(T::WeightInfo::leave_intent(T::MaxCollators::get()))] // todo:collator weight
+ pub fn force_revoke_license(
origin: OriginFor<T>,
who: T::AccountId,
) -> DispatchResultWithPostInfo {
// leave_intent
T::UpdateOrigin::ensure_origin(origin)?;
- let current_count = Self::try_remove_candidate(&who)?;
- Self::try_release_license(&who, false)?;
+ let current_count = Self::try_remove_candidate_and_release_license(&who, false, true)?;
Ok(Some(T::WeightInfo::leave_intent(current_count as u32)).into()) // todo:collator weight
}
@@ -636,6 +606,23 @@
T::PotId::get().into_account_truncating()
}
+ fn try_remove_candidate_and_release_license(
+ who: &T::AccountId,
+ should_slash: bool,
+ ignore_if_not_candidate: bool,
+ ) -> Result<usize, DispatchError> {
+ let current_count = Self::try_remove_candidate(who);
+ let current_count = if ignore_if_not_candidate
+ && current_count == Err(Error::<T>::NotCandidate.into())
+ {
+ <Candidates<T>>::decode_len().unwrap_or_default()
+ } else {
+ current_count?
+ };
+ Self::try_release_license(who, should_slash)?;
+ Ok(current_count)
+ }
+
/// Removes a candidate from the collator pool for the next session if they exist.
fn try_remove_candidate(who: &T::AccountId) -> Result<usize, DispatchError> {
let current_count =
@@ -657,7 +644,7 @@
/// Removes a candidate if they exist and sends them back their deposit, optionally slashed.
fn try_release_license(who: &T::AccountId, should_slash: bool) -> DispatchResult {
let mut deposit_returned = BalanceOf::<T>::default();
- Licenses::<T>::try_mutate_exists(&who, |deposit| -> DispatchResult {
+ Licenses::<T>::try_mutate_exists(who, |deposit| -> DispatchResult {
if let Some(deposit) = deposit.take() {
if should_slash {
let slashed = T::SlashRatio::get() * deposit;
@@ -690,7 +677,7 @@
///
/// This is done on the fly, as frequent as we are told to do so, as the session manager.
pub fn assemble_collators(
- candidates: BoundedVec<T::AccountId, T::MaxCandidates>,
+ candidates: BoundedVec<T::AccountId, T::MaxCollators>,
) -> Vec<T::AccountId> {
let mut collators = Self::invulnerables().to_vec();
collators.extend(candidates);
@@ -700,8 +687,8 @@
/// Kicks out candidates that did not produce a block in the kick threshold
/// and **confiscates** their deposits to the treasury.
pub fn kick_stale_candidates(
- candidates: BoundedVec<T::AccountId, T::MaxCandidates>, //LicenseInfo<T::AccountId, BalanceOf<T>>
- ) -> BoundedVec<T::AccountId, T::MaxCandidates> {
+ candidates: BoundedVec<T::AccountId, T::MaxCollators>, //LicenseInfo<T::AccountId, BalanceOf<T>>
+ ) -> BoundedVec<T::AccountId, T::MaxCollators> {
let now = frame_system::Pallet::<T>::block_number();
let kick_threshold = Self::kick_threshold();
candidates
@@ -709,21 +696,13 @@
.filter_map(|c| {
let last_block = <LastAuthoredBlock<T>>::get(c.clone());
let since_last = now.saturating_sub(last_block);
- if since_last < kick_threshold ||
- Self::candidates().len() as u32 <= T::MinCandidates::get()
- {
+ if since_last < kick_threshold {
Some(c)
} else {
- let outcome = Self::try_remove_candidate(&c);
+ let outcome = Self::try_remove_candidate_and_release_license(&c, true, false);
if let Err(why) = outcome {
- log::warn!("Failed to remove candidate {:?}", why);
- debug_assert!(false, "failed to remove candidate {:?}", why);
- return None;
- }
- let outcome = Self::try_release_license(&c, true);
- if let Err(why) = outcome {
- log::warn!("Failed to release license {:?}", why);
- debug_assert!(false, "failed to release license {:?}", why);
+ log::warn!("Failed to kick collator and release license {:?}", why);
+ debug_assert!(false, "failed to kick collator and release license {why:?}");
}
None
}
pallets/collator-selection/src/mock.rsdiffbeforeafterboth--- a/pallets/collator-selection/src/mock.rs
+++ b/pallets/collator-selection/src/mock.rs
@@ -206,9 +206,7 @@
parameter_types! {
pub const PotId: PalletId = PalletId(*b"PotStake");
- pub const MaxCandidates: u32 = 20;
- pub const MaxInvulnerables: u32 = 20;
- pub const MinCandidates: u32 = 1;
+ pub const MaxCollators: u32 = 20;
pub const MaxAuthorities: u32 = 100_000;
pub const SlashRatio: Perbill = Perbill::one();
}
@@ -230,9 +228,7 @@
type Currency = Balances;
type UpdateOrigin = EnsureSignedBy<RootAccount, u64>;
type PotId = PotId;
- type MaxCandidates = MaxCandidates;
- type MinCandidates = MinCandidates;
- type MaxInvulnerables = MaxInvulnerables;
+ type MaxCollators = MaxCollators;
// type KickThreshold = Period;
type SlashRatio = SlashRatio;
type TreasuryAccountId = ();
@@ -263,9 +259,9 @@
})
.collect::<Vec<_>>();
let collator_selection = collator_selection::GenesisConfig::<Test> {
- desired_candidates: 2,
+ desired_collators: 5,
license_bond: 10,
- kick_threshold: 1,
+ kick_threshold: 10,
invulnerables,
};
let session = pallet_session::GenesisConfig::<Test> { keys };
pallets/collator-selection/src/tests.rsdiffbeforeafterboth--- a/pallets/collator-selection/src/tests.rs
+++ b/pallets/collator-selection/src/tests.rs
@@ -31,7 +31,7 @@
// limitations under the License.
use crate as collator_selection;
-use crate::{mock::*, LicenseInfo, Error};
+use crate::{mock::*, Error};
use frame_support::{
assert_noop, assert_ok,
traits::{Currency, GenesisBuild, OnInitialize},
@@ -39,10 +39,19 @@
use pallet_balances::Error as BalancesError;
use sp_runtime::traits::BadOrigin;
+fn get_license_and_onboard(account_id: <Test as frame_system::Config>::AccountId) {
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(
+ account_id
+ )));
+ assert_ok!(CollatorSelection::onboard(RuntimeOrigin::signed(
+ account_id
+ )));
+}
+
#[test]
fn basic_setup_works() {
new_test_ext().execute_with(|| {
- assert_eq!(CollatorSelection::desired_candidates(), 2);
+ assert_eq!(CollatorSelection::desired_collators(), 5);
assert_eq!(CollatorSelection::license_bond(), 10);
assert!(CollatorSelection::candidates().is_empty());
@@ -51,6 +60,7 @@
}
// todo:collator add more tests later
+// invulnerable after onboard + invulnerables can bypass desired_candidates
#[test]
fn it_should_add_invulnerables() {
@@ -112,21 +122,21 @@
}
#[test]
-fn set_desired_candidates_works() {
+fn set_desired_collators_works() {
new_test_ext().execute_with(|| {
// given
- assert_eq!(CollatorSelection::desired_candidates(), 2);
+ assert_eq!(CollatorSelection::desired_collators(), 5);
// can set
- assert_ok!(CollatorSelection::set_desired_candidates(
+ assert_ok!(CollatorSelection::set_desired_collators(
RuntimeOrigin::signed(RootAccount::get()),
7
));
- assert_eq!(CollatorSelection::desired_candidates(), 7);
+ assert_eq!(CollatorSelection::desired_collators(), 7);
// rejects bad origin
assert_noop!(
- CollatorSelection::set_desired_candidates(RuntimeOrigin::signed(1), 8),
+ CollatorSelection::set_desired_collators(RuntimeOrigin::signed(1), 8),
BadOrigin
);
});
@@ -154,166 +164,246 @@
}
#[test]
-fn cannot_register_candidate_if_too_many() {
+fn cannot_onboard_candidate_with_no_license() {
new_test_ext().execute_with(|| {
- // reset desired candidates:
- <crate::DesiredCandidates<Test>>::put(0);
+ // can't onboard a candidate who did not get a license.
+ assert_noop!(
+ CollatorSelection::onboard(RuntimeOrigin::signed(3)),
+ Error::<Test>::NoLicense,
+ );
+
+ // but give it a license and welcome aboard.
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(3)));
+ assert_ok!(CollatorSelection::onboard(RuntimeOrigin::signed(3)));
+ })
+}
+
+#[test]
+fn cannot_onboard_candidate_if_too_many() {
+ new_test_ext().execute_with(|| {
+ // reset desired candidates
+ <crate::DesiredCollators<Test>>::put(0);
+
+ // can still get a license.
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(4)));
// can't accept anyone anymore.
assert_noop!(
- CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)),
+ CollatorSelection::onboard(RuntimeOrigin::signed(4)),
Error::<Test>::TooManyCandidates,
);
- // reset desired candidates:
- <crate::DesiredCandidates<Test>>::put(1);
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(4)
- ));
+ // reset desired candidates to invulnerables + 1
+ <crate::DesiredCollators<Test>>::put(3);
+ assert_ok!(CollatorSelection::onboard(RuntimeOrigin::signed(4)));
- // but no more
+ // but no more.
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(5)));
assert_noop!(
- CollatorSelection::register_as_candidate(RuntimeOrigin::signed(5)),
+ CollatorSelection::onboard(RuntimeOrigin::signed(5)),
Error::<Test>::TooManyCandidates,
);
})
}
#[test]
-fn cannot_unregister_candidate_if_too_few() {
+fn cannot_obtain_license_if_keys_not_registered() {
new_test_ext().execute_with(|| {
- // reset desired candidates:
- <crate::DesiredCandidates<Test>>::put(1);
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(4)
- ));
-
- // can not remove too few
+ // can't 7 because keys not registered.
assert_noop!(
- CollatorSelection::leave_intent(RuntimeOrigin::signed(4)),
- Error::<Test>::TooFewCandidates,
+ CollatorSelection::get_license(RuntimeOrigin::signed(7)),
+ Error::<Test>::ValidatorNotRegistered
);
})
}
#[test]
-fn cannot_register_as_candidate_if_invulnerable() {
+fn cannot_obtain_license_if_poor() {
new_test_ext().execute_with(|| {
- assert_eq!(CollatorSelection::invulnerables(), vec![1, 2]);
+ assert_eq!(Balances::free_balance(&3), 100);
+ assert_eq!(Balances::free_balance(&33), 0);
- // can't 1 because it is invulnerable.
- assert_noop!(
- CollatorSelection::register_as_candidate(RuntimeOrigin::signed(1)),
- Error::<Test>::AlreadyInvulnerable,
- );
- })
-}
+ // works
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(3)));
-#[test]
-fn cannot_register_as_candidate_if_keys_not_registered() {
- new_test_ext().execute_with(|| {
- // can't 7 because keys not registered.
+ // poor
assert_noop!(
- CollatorSelection::register_as_candidate(RuntimeOrigin::signed(7)),
- Error::<Test>::ValidatorNotRegistered
+ CollatorSelection::get_license(RuntimeOrigin::signed(33)),
+ BalancesError::<Test>::InsufficientBalance,
);
- })
+ });
}
#[test]
-fn cannot_register_dupe_candidate() {
+fn cannot_onboard_dupe_candidate() {
new_test_ext().execute_with(|| {
// can add 3 as candidate
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(3)
- ));
- let addition = LicenseInfo {
- who: 3,
- deposit: 10,
- };
- assert_eq!(CollatorSelection::candidates(), vec![addition]);
+ get_license_and_onboard(3);
+ assert_eq!(CollatorSelection::licenses(3), 10);
+ assert_eq!(CollatorSelection::candidates(), vec![3]);
assert_eq!(CollatorSelection::last_authored_block(3), 10);
assert_eq!(Balances::free_balance(3), 90);
// but no more
assert_noop!(
- CollatorSelection::register_as_candidate(RuntimeOrigin::signed(3)),
+ CollatorSelection::get_license(RuntimeOrigin::signed(3)),
+ Error::<Test>::AlreadyHoldingLicense,
+ );
+ assert_noop!(
+ CollatorSelection::onboard(RuntimeOrigin::signed(3)),
Error::<Test>::AlreadyCandidate,
);
})
}
#[test]
-fn cannot_register_as_candidate_if_poor() {
+fn becoming_candidate_works() {
new_test_ext().execute_with(|| {
+ // given
+ assert_eq!(CollatorSelection::desired_collators(), 5);
+ assert_eq!(CollatorSelection::license_bond(), 10);
+ assert_eq!(CollatorSelection::candidates(), Vec::new());
+ assert_eq!(CollatorSelection::invulnerables(), vec![1, 2]);
+
+ // take two endowed, non-invulnerables accounts.
assert_eq!(Balances::free_balance(&3), 100);
- assert_eq!(Balances::free_balance(&33), 0);
+ assert_eq!(Balances::free_balance(&4), 100);
- // works
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(3)
- ));
+ get_license_and_onboard(3);
+ get_license_and_onboard(4);
+
+ assert_eq!(Balances::free_balance(&3), 90);
+ assert_eq!(Balances::free_balance(&4), 90);
- // poor
- assert_noop!(
- CollatorSelection::register_as_candidate(RuntimeOrigin::signed(33)),
- BalancesError::<Test>::InsufficientBalance,
- );
+ assert_eq!(CollatorSelection::candidates().len(), 2);
});
}
#[test]
-fn register_as_candidate_works() {
+fn cannot_become_candidate_if_invulnerable() {
new_test_ext().execute_with(|| {
- // given
- assert_eq!(CollatorSelection::desired_candidates(), 2);
- assert_eq!(CollatorSelection::license_bond(), 10);
- assert_eq!(CollatorSelection::candidates(), Vec::new());
assert_eq!(CollatorSelection::invulnerables(), vec![1, 2]);
- // take two endowed, non-invulnerables accounts.
- assert_eq!(Balances::free_balance(&3), 100);
- assert_eq!(Balances::free_balance(&4), 100);
+ // can obtain a license even if is invulnerable.
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(1)));
+ // but cannot onboard
+ assert_noop!(
+ CollatorSelection::onboard(RuntimeOrigin::signed(1)),
+ Error::<Test>::AlreadyInvulnerable,
+ );
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(3)
+ // get a license and then become invulnerable.
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(3)));
+ assert_ok!(CollatorSelection::add_invulnerable(
+ RuntimeOrigin::signed(RootAccount::get()),
+ 3
));
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(4)
+ assert_noop!(
+ CollatorSelection::onboard(RuntimeOrigin::signed(3)),
+ Error::<Test>::AlreadyInvulnerable,
+ );
+ })
+}
+
+#[test]
+fn can_become_invulnerable_if_candidate() {
+ new_test_ext().execute_with(|| {
+ // become a candidate and then become invulnerable.
+ get_license_and_onboard(3);
+ assert_eq!(CollatorSelection::candidates(), vec![3]);
+
+ assert_ok!(CollatorSelection::add_invulnerable(
+ RuntimeOrigin::signed(RootAccount::get()),
+ 3
));
+ // should exclude from candidates, but not revoke the license
+ assert_eq!(CollatorSelection::candidates(), vec![]);
+ assert_eq!(CollatorSelection::licenses(3), 10);
+ assert_eq!(Balances::free_balance(3), 90);
+ });
+}
- assert_eq!(Balances::free_balance(&3), 90);
- assert_eq!(Balances::free_balance(&4), 90);
+#[test]
+fn offboard() {
+ new_test_ext().execute_with(|| {
+ // register a candidate.
+ get_license_and_onboard(3);
+ assert_eq!(Balances::free_balance(3), 90);
+
+ // cannot leave if holds license but not yet candidate.
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(4)));
+ assert_noop!(
+ CollatorSelection::offboard(RuntimeOrigin::signed(4)),
+ Error::<Test>::NotCandidate
+ );
+ // cannot leave if does not hold license.
+ assert_noop!(
+ CollatorSelection::offboard(RuntimeOrigin::signed(5)),
+ Error::<Test>::NotCandidate
+ );
- assert_eq!(CollatorSelection::candidates().len(), 2);
+ // bond is returned - only after releasing the license
+ assert_ok!(CollatorSelection::offboard(RuntimeOrigin::signed(3)));
+ assert_eq!(Balances::free_balance(3), 90);
+ assert_eq!(CollatorSelection::last_authored_block(3), 0);
+ assert_ok!(CollatorSelection::release_license(RuntimeOrigin::signed(3)));
+ assert_eq!(Balances::free_balance(3), 100);
});
}
#[test]
-fn leave_intent() {
+fn release_license() {
new_test_ext().execute_with(|| {
+ // obtain a license to collate and reserve the bond.
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(3)));
+ assert_eq!(Balances::free_balance(3), 90);
+
+ // release the license and get the bond back.
+ assert_ok!(CollatorSelection::release_license(RuntimeOrigin::signed(3)));
+ assert_eq!(Balances::free_balance(3), 100);
+
// register a candidate.
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(3)
- ));
+ get_license_and_onboard(3);
assert_eq!(Balances::free_balance(3), 90);
- // register too so can leave above min candidates
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(5)
- ));
- assert_eq!(Balances::free_balance(5), 90);
+ // can release license even if onboarded.
+ assert_ok!(CollatorSelection::release_license(RuntimeOrigin::signed(3)));
+ assert_eq!(Balances::free_balance(3), 100);
+ assert_eq!(CollatorSelection::candidates(), vec![]);
+ });
+}
+
+#[test]
+fn force_revoke_license() {
+ new_test_ext().execute_with(|| {
+ // obtain a license to collate and reserve the bond.
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(3)));
+ assert_eq!(Balances::free_balance(3), 90);
- // cannot leave if not candidate.
+ // cannot execute the operation as non-root
assert_noop!(
- CollatorSelection::leave_intent(RuntimeOrigin::signed(4)),
- Error::<Test>::NotCandidate
+ CollatorSelection::force_revoke_license(RuntimeOrigin::signed(3), 3),
+ BadOrigin
);
- // bond is returned
- assert_ok!(CollatorSelection::leave_intent(RuntimeOrigin::signed(3)));
+ // release the license and get the bond back.
+ assert_ok!(CollatorSelection::force_revoke_license(
+ RuntimeOrigin::signed(RootAccount::get()),
+ 3
+ ));
+ assert_eq!(Balances::free_balance(3), 100);
+
+ // register a candidate.
+ get_license_and_onboard(3);
+ assert_eq!(Balances::free_balance(3), 90);
+
+ // can release license even if onboarded.
+ assert_ok!(CollatorSelection::force_revoke_license(
+ RuntimeOrigin::signed(RootAccount::get()),
+ 3
+ ));
assert_eq!(Balances::free_balance(3), 100);
- assert_eq!(CollatorSelection::last_authored_block(3), 0);
+ assert_eq!(CollatorSelection::candidates(), vec![]);
});
}
@@ -325,18 +415,11 @@
// 4 is the default author.
assert_eq!(Balances::free_balance(4), 100);
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(4)
- ));
+ get_license_and_onboard(4);
// triggers `note_author`
Authorship::on_initialize(1);
- let collator = LicenseInfo {
- who: 4,
- deposit: 10,
- };
-
- assert_eq!(CollatorSelection::candidates(), vec![collator]);
+ assert_eq!(CollatorSelection::candidates(), vec![4]);
assert_eq!(CollatorSelection::last_authored_block(4), 0);
// half of the pot goes to the collator who's the author (4 in tests).
@@ -355,18 +438,11 @@
Balances::make_free_balance_be(&CollatorSelection::account_id(), 5);
// 4 is the default author.
assert_eq!(Balances::free_balance(4), 100);
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(4)
- ));
+ get_license_and_onboard(4);
// triggers `note_author`
Authorship::on_initialize(1);
- let collator = LicenseInfo {
- who: 4,
- deposit: 10,
- };
-
- assert_eq!(CollatorSelection::candidates(), vec![collator]);
+ assert_eq!(CollatorSelection::candidates(), vec![4]);
assert_eq!(CollatorSelection::last_authored_block(4), 0);
// Nothing received
assert_eq!(Balances::free_balance(4), 90);
@@ -389,9 +465,7 @@
assert_eq!(SessionHandlerCollators::get(), vec![1, 2]);
// add a new collator
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(3)
- ));
+ get_license_and_onboard(5);
// session won't see this.
assert_eq!(SessionHandlerCollators::get(), vec![1, 2]);
@@ -410,7 +484,7 @@
initialize_to_block(20);
assert_eq!(SessionChangeBlock::get(), 20);
// changed are now reflected to session handlers.
- assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 3]);
+ assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 5]);
});
}
@@ -418,64 +492,28 @@
fn kick_mechanism() {
new_test_ext().execute_with(|| {
// add a new collator
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(3)
- ));
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(4)
- ));
+ get_license_and_onboard(3);
+ get_license_and_onboard(4);
+
initialize_to_block(10);
assert_eq!(CollatorSelection::candidates().len(), 2);
+
initialize_to_block(20);
assert_eq!(SessionChangeBlock::get(), 20);
// 4 authored this block, gets to stay 3 was kicked
assert_eq!(CollatorSelection::candidates().len(), 1);
// 3 will be kicked after 1 session delay
assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 3, 4]);
- let collator = LicenseInfo {
- who: 4,
- deposit: 10,
- };
- assert_eq!(CollatorSelection::candidates(), vec![collator]);
- assert_eq!(CollatorSelection::kick_threshold(), 1);
+
+ assert_eq!(CollatorSelection::candidates(), vec![4]);
+ assert_eq!(CollatorSelection::kick_threshold(), 10);
assert_eq!(CollatorSelection::last_authored_block(4), 20);
+
initialize_to_block(30);
// 3 gets kicked after 1 session delay
assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 4]);
- // kicked collator gets funds back
- assert_eq!(Balances::free_balance(3), 100);
- });
-}
-
-#[test]
-fn should_not_kick_mechanism_too_few() {
- new_test_ext().execute_with(|| {
- // add a new collator
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(3)
- ));
- assert_ok!(CollatorSelection::register_as_candidate(
- RuntimeOrigin::signed(5)
- ));
- initialize_to_block(10);
- assert_eq!(CollatorSelection::candidates().len(), 2);
- initialize_to_block(20);
- assert_eq!(SessionChangeBlock::get(), 20);
- // 4 authored this block, 5 gets to stay too few 3 was kicked
- assert_eq!(CollatorSelection::candidates().len(), 1);
- // 3 will be kicked after 1 session delay
- assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 3, 5]);
- let collator = LicenseInfo {
- who: 5,
- deposit: 10,
- };
- assert_eq!(CollatorSelection::candidates(), vec![collator]);
- assert_eq!(CollatorSelection::last_authored_block(4), 20);
- initialize_to_block(30);
- // 3 gets kicked after 1 session delay
- assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 5]);
- // kicked collator gets funds back
- assert_eq!(Balances::free_balance(3), 100);
+ // kicked collator gets their funds slashed, the deposit going to treasury
+ assert_eq!(Balances::free_balance(3), 90);
});
}
@@ -489,9 +527,9 @@
let invulnerables = vec![1, 1];
let collator_selection = collator_selection::GenesisConfig::<Test> {
- desired_candidates: 2,
+ desired_collators: 5,
license_bond: 10,
- kick_threshold: 1,
+ kick_threshold: 10,
invulnerables,
};
// collator selection must be initialized before session.
pallets/collator-selection/src/weights.rsdiffbeforeafterboth--- a/pallets/collator-selection/src/weights.rs
+++ b/pallets/collator-selection/src/weights.rs
@@ -45,7 +45,7 @@
// The weight info trait for `pallet_collator_selection`.
pub trait WeightInfo {
fn set_invulnerables(_b: u32) -> Weight;
- fn set_desired_candidates() -> Weight;
+ fn set_desired_collators() -> Weight;
fn set_license_bond() -> Weight;
fn register_as_candidate(_c: u32) -> Weight;
fn leave_intent(_c: u32) -> Weight;
@@ -62,7 +62,7 @@
.saturating_add(Weight::from_ref_time(68_000 as u64).saturating_mul(b as u64))
.saturating_add(T::DbWeight::get().writes(1 as u64))
}
- fn set_desired_candidates() -> Weight {
+ fn set_desired_collators() -> Weight {
Weight::from_ref_time(16_363_000 as u64).saturating_add(T::DbWeight::get().writes(1 as u64))
}
fn set_license_bond() -> Weight {
@@ -108,7 +108,7 @@
.saturating_add(Weight::from_ref_time(68_000 as u64).saturating_mul(b as u64))
.saturating_add(RocksDbWeight::get().writes(1 as u64))
}
- fn set_desired_candidates() -> Weight {
+ fn set_desired_collators() -> Weight {
Weight::from_ref_time(16_363_000 as u64)
.saturating_add(RocksDbWeight::get().writes(1 as u64))
}
primitives/common/src/constants.rsdiffbeforeafterboth--- a/primitives/common/src/constants.rs
+++ b/primitives/common/src/constants.rs
@@ -45,9 +45,9 @@
/// Minimum balance required to create or keep an account open.
pub const EXISTENTIAL_DEPOSIT: u128 = 0;
/// Amount of Balance reserved for candidate registration.
-pub const GENESIS_LICENSE_BOND: u128 = EXISTENTIAL_DEPOSIT;
+pub const GENESIS_LICENSE_BOND: u128 = 1_000_000_000_000 * UNIQUE;
/// How long a periodic session lasts in blocks.
-pub const SESSION_LENGTH: BlockNumber = MINUTES;
+pub const SESSION_LENGTH: BlockNumber = HOURS;
// Targeting 0.1 UNQ per transfer
pub const WEIGHT_TO_FEE_COEFF: u32 = /*<weight2fee>*/175_199_920/*</weight2fee>*/;
runtime/common/config/pallets/collator_selection.rsdiffbeforeafterboth--- a/runtime/common/config/pallets/collator_selection.rs
+++ b/runtime/common/config/pallets/collator_selection.rs
@@ -55,9 +55,7 @@
parameter_types! {
pub const PotId: PalletId = PalletId(*b"PotStake");
- pub const MaxCandidates: u32 = 30; // todo:collator 30 collator slots - 3 planned invulnerables
- pub const MinCandidates: u32 = 1;
- pub const MaxInvulnerables: u32 = 30;
+ pub const MaxCollators: u32 = 10;
pub const SlashRatio: Perbill = Perbill::from_percent(100);
}
@@ -68,9 +66,7 @@
type UpdateOrigin = EnsureRoot<AccountId>;
type TreasuryAccountId = TreasuryAccountId;
type PotId = PotId;
- type MaxCandidates = MaxCandidates;
- type MinCandidates = MinCandidates;
- type MaxInvulnerables = MaxInvulnerables;
+ type MaxCollators = MaxCollators;
// todo:collator kick threshold should be in storage and configured only by root -- or rather UpdateOrigin
type SlashRatio = SlashRatio;
type ValidatorId = <Self as frame_system::Config>::AccountId;
runtime/common/mod.rsdiffbeforeafterboth--- a/runtime/common/mod.rs
+++ b/runtime/common/mod.rs
@@ -192,7 +192,7 @@
};
use pallet_session::SessionManager;
use up_common::constants::GENESIS_LICENSE_BOND;
- use crate::config::pallets::collator_selection::MaxInvulnerables;
+ use crate::config::pallets::collator_selection::MaxCollators;
let mut weight = <Runtime as frame_system::Config>::DbWeight::get().reads(1);
@@ -231,17 +231,17 @@
})
.collect::<Vec<_>>();
- let bounded_invulnerables = BoundedVec::<_, MaxInvulnerables>::try_from(
+ let bounded_invulnerables = BoundedVec::<_, MaxCollators>::try_from(
invulnerables
.iter()
.cloned()
.map(|(acc, _)| acc)
.collect::<Vec<_>>(),
)
- .expect("Existing collators/invulnerables are more than MaxInvulnerables");
+ .expect("Existing collators/invulnerables are more than MaxCollators");
<pallet_collator_selection::Invulnerables<Runtime>>::put(bounded_invulnerables);
- <pallet_collator_selection::DesiredCandidates<Runtime>>::put(0);
+ <pallet_collator_selection::DesiredCollators<Runtime>>::put(MaxCollators::get());
<pallet_collator_selection::LicenseBond<Runtime>>::put(GENESIS_LICENSE_BOND);
let keys = invulnerables
runtime/common/tests/mod.rsdiffbeforeafterboth--- a/runtime/common/tests/mod.rs
+++ b/runtime/common/tests/mod.rs
@@ -18,7 +18,7 @@
use sp_core::{Public, Pair};
use sp_std::vec;
use up_common::types::AuraId;
-use crate::{GenesisConfig, ParachainInfoConfig, AuraConfig};
+use crate::{GenesisConfig, ParachainInfoConfig};
pub mod xcm;
@@ -28,7 +28,61 @@
.public()
}
+#[cfg(feature = "collator-selection")]
+fn new_test_ext(para_id: u32) -> sp_io::TestExternalities {
+ use sp_core::{sr25519};
+ use sp_runtime::traits::{IdentifyAccount, Verify};
+ use crate::{AccountId, Signature, SessionKeys, CollatorSelectionConfig, SessionConfig};
+
+ type AccountPublic = <Signature as Verify>::Signer;
+
+ fn get_account_id_from_seed<TPublic: Public>(seed: &str) -> AccountId
+ where
+ AccountPublic: From<<TPublic::Pair as Pair>::Public>,
+ {
+ AccountPublic::from(get_from_seed::<TPublic>(seed)).into_account()
+ }
+
+ let accounts = vec!["Alice", "Bob"];
+ let keys = accounts
+ .iter()
+ .map(|&acc| {
+ let account_id = get_account_id_from_seed::<sr25519::Public>(acc);
+ (
+ account_id.clone(),
+ account_id,
+ SessionKeys {
+ aura: get_from_seed::<AuraId>(acc),
+ },
+ )
+ })
+ .collect::<Vec<_>>();
+ let invulnerables = accounts
+ .iter()
+ .map(|acc| get_account_id_from_seed::<sr25519::Public>(acc))
+ .collect::<Vec<_>>();
+
+ let cfg = GenesisConfig {
+ collator_selection: CollatorSelectionConfig {
+ desired_collators: 2,
+ license_bond: 10,
+ kick_threshold: 10,
+ invulnerables,
+ },
+ session: SessionConfig { keys },
+ parachain_info: ParachainInfoConfig {
+ parachain_id: para_id.into(),
+ },
+ ..GenesisConfig::default()
+ };
+
+ cfg.build_storage().unwrap().into()
+}
+
+#[cfg(not(feature = "collator-selection"))]
fn new_test_ext(para_id: u32) -> sp_io::TestExternalities {
+ use crate::AuraConfig;
+
let cfg = GenesisConfig {
aura: AuraConfig {
authorities: vec![