difftreelog
refactor! decouple pallet-collator-selection from pallet-configuration (#962)
in: master
* feat!: decoupling configuration&collators pallets BREAKING CHANGE: pallet `collator-selection` no longer has a tight coupling with the `configuration` pallet * refactor(collator-selection): type bounds, tests * refactor(collator-selection): tests * add ci step for banchmark tests * add opsFee tests ---------
15 files changed
.github/workflows/yarn-dev.ymldiffbeforeafterboth--- a/.github/workflows/yarn-dev.yml
+++ b/.github/workflows/yarn-dev.yml
@@ -87,6 +87,18 @@
run: |
echo "url is ${{ steps.test-report.outputs.runHtmlUrl }}"
+ - name: Run benchmark mintFee tests
+ working-directory: tests
+ run: |
+ yarn install
+ npx ts-node --esm ./src/benchmarks/mintFee/index.ts
+
+ - name: Run benchmark opsFee tests
+ working-directory: tests
+ run: |
+ yarn install
+ npx ts-node --esm ./src/benchmarks/opsFee/index.ts
+
- name: Stop running containers
if: always() # run this step always
run: docker-compose -f ".docker/docker-compose.${{ matrix.network }}.yml" down
Cargo.lockdiffbeforeafterboth--- a/Cargo.lock
+++ b/Cargo.lock
@@ -6371,7 +6371,7 @@
[[package]]
name = "pallet-collator-selection"
-version = "4.0.0"
+version = "5.0.0"
dependencies = [
"frame-benchmarking",
"frame-support",
@@ -6380,7 +6380,6 @@
"pallet-aura",
"pallet-authorship",
"pallet-balances",
- "pallet-configuration",
"pallet-session",
"pallet-timestamp",
"parity-scale-codec",
@@ -6435,7 +6434,7 @@
[[package]]
name = "pallet-configuration"
-version = "0.1.3"
+version = "0.2.0"
dependencies = [
"fp-evm",
"frame-benchmarking",
pallets/collator-selection/CHANGELOG.mddiffbeforeafterboth--- a/pallets/collator-selection/CHANGELOG.md
+++ b/pallets/collator-selection/CHANGELOG.md
@@ -4,22 +4,28 @@
<!-- bureaucracy goes here -->
+## [5.0.0] - 2023-06-27
+
+### Major change
+
+- The dependency (tight coupling) this pallet on the configuration pallet has been removed.
+
## [4.0.0] - 2022-12-29
### Added
- Entire functionality moved over from cumulus/pallet-collator-selection (v3.0.0). Refactored business logic:
- - Added an extra step for candidacy, `get_license`, at which payment happens. The number of license holders is unlimited.
- - Only licensed accounts can apply for candidacy with `onboard`. No extra deposits are made.
- - Active candidates may `offboard`, but they will retain their license.
- - Deposit is returned, and candidacy possibly removed with `release_license`.
- - License can be forcibly forfeited and candidacy removed with `force_release_license`.
- - Failing collators' deposits will now be fully slashed, and funds will be redirected to Treasury.
- - Unify `MaxInvulnerables` and `MaxCandidates` into `MaxCollators`.
- - Remove `MinCandidates`.
- - Minimal amount of invulnerables is now 1.
- - Both invulnerables and candidates count against the limits together, however invulnerables ignore `DesiredCollators`.
- - `KickThreshold` is made configurable.
- - `DesiredCollators`, `LicenseBond`, and `KickThreshold` are moved to and referenced from `pallet-configuration`.
- - Naming changes to better reflect the new functionality.
- - More minor changes, tests, benchmarks, etc.
\ No newline at end of file
+ - Added an extra step for candidacy, `get_license`, at which payment happens. The number of license holders is unlimited.
+ - Only licensed accounts can apply for candidacy with `onboard`. No extra deposits are made.
+ - Active candidates may `offboard`, but they will retain their license.
+ - Deposit is returned, and candidacy possibly removed with `release_license`.
+ - License can be forcibly forfeited and candidacy removed with `force_release_license`.
+ - Failing collators' deposits will now be fully slashed, and funds will be redirected to Treasury.
+ - Unify `MaxInvulnerables` and `MaxCandidates` into `MaxCollators`.
+ - Remove `MinCandidates`.
+ - Minimal amount of invulnerables is now 1.
+ - Both invulnerables and candidates count against the limits together, however invulnerables ignore `DesiredCollators`.
+ - `KickThreshold` is made configurable.
+ - `DesiredCollators`, `LicenseBond`, and `KickThreshold` are moved to and referenced from `pallet-configuration`.
+ - Naming changes to better reflect the new functionality.
+ - More minor changes, tests, benchmarks, etc.
pallets/collator-selection/Cargo.tomldiffbeforeafterboth--- a/pallets/collator-selection/Cargo.toml
+++ b/pallets/collator-selection/Cargo.toml
@@ -6,7 +6,7 @@
license = "GPLv3"
name = "pallet-collator-selection"
repository = "https://github.com/UniqueNetwork/unique-chain"
-version = "4.0.0"
+version = "5.0.0"
[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]
@@ -23,7 +23,6 @@
frame-support = { workspace = true }
frame-system = { workspace = true }
pallet-authorship = { workspace = true }
-pallet-configuration = { workspace = true }
pallet-session = { workspace = true }
sp-runtime = { workspace = true }
sp-staking = { workspace = true }
@@ -55,8 +54,6 @@
"frame-system/std",
"log/std",
"pallet-authorship/std",
- 'pallet-aura/std',
- 'pallet-balances/std',
"pallet-session/std",
"rand/std",
"scale-info/std",
@@ -64,6 +61,8 @@
"sp-runtime/std",
"sp-staking/std",
"sp-std/std",
+ 'pallet-aura/std',
+ 'pallet-balances/std',
]
try-runtime = ["frame-support/try-runtime"]
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::{44 EnsureOrigin,45 fungible::{Inspect, Mutate},46 Get,47 },48};49use frame_system::{EventRecord, RawOrigin};50use pallet_authorship::EventHandler;51use pallet_session::{self as session, SessionManager};52use pallet_configuration::{53 self as configuration, BalanceOf,54 CollatorSelectionDesiredCollatorsOverride as DesiredCollators,55 CollatorSelectionLicenseBondOverride as LicenseBond,56};57use sp_std::prelude::*;5859const SEED: u32 = 0;6061// TODO: remove if this is given in substrate commit.62macro_rules! whitelist {63 ($acc:ident) => {64 frame_benchmarking::benchmarking::add_to_whitelist(65 frame_system::Account::<T>::hashed_key_for(&$acc).into(),66 );67 };68}6970fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {71 let events = frame_system::Pallet::<T>::events();72 let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();73 // compare to the last event record74 let EventRecord { event, .. } = &events[events.len() - 1];75 assert_eq!(event, &system_event);76}7778fn create_funded_user<T: Config>(79 string: &'static str,80 n: u32,81 balance_factor: u32,82) -> T::AccountId {83 let user = account(string, n, SEED);84 let balance = balance_unit::<T>() * balance_factor.into();85 let _ = T::Currency::set_balance(&user, balance);86 user87}8889fn keys<T: Config + session::Config>(c: u32) -> <T as session::Config>::Keys {90 use rand::{RngCore, SeedableRng};9192 let keys = {93 let mut keys = [0u8; 128];9495 if c > 0 {96 let mut rng = rand::rngs::StdRng::seed_from_u64(c as u64);97 rng.fill_bytes(&mut keys);98 }99100 keys101 };102103 Decode::decode(&mut &keys[..]).unwrap()104}105106fn validator<T: Config + session::Config>(c: u32) -> (T::AccountId, <T as session::Config>::Keys) {107 (create_funded_user::<T>("candidate", c, 1000), keys::<T>(c))108}109110fn register_validators<T: Config + session::Config>(count: u32) -> Vec<T::AccountId> {111 let validators = (0..count).map(|c| validator::<T>(c)).collect::<Vec<_>>();112113 for (who, keys) in validators.clone() {114 <session::Pallet<T>>::set_keys(RawOrigin::Signed(who).into(), keys, Vec::new()).unwrap();115 }116117 validators.into_iter().map(|(who, _)| who).collect()118}119120fn register_invulnerables<T: Config + configuration::Config>(count: u32) {121 let candidates = (0..count)122 .map(|c| account("candidate", c, SEED))123 .collect::<Vec<_>>();124125 for who in candidates {126 <CollatorSelection<T>>::add_invulnerable(127 T::UpdateOrigin::try_successful_origin().unwrap(),128 who,129 )130 .unwrap();131 }132}133134fn register_candidates<T: Config + configuration::Config>(count: u32) {135 let candidates = (0..count)136 .map(|c| account("candidate", c, SEED))137 .collect::<Vec<_>>();138 assert!(139 <LicenseBond<T>>::get() > 0u32.into(),140 "Bond cannot be zero!"141 );142143 for who in candidates {144 T::Currency::set_balance(&who, <LicenseBond<T>>::get() * 2u32.into());145 <CollatorSelection<T>>::get_license(RawOrigin::Signed(who.clone()).into()).unwrap();146 <CollatorSelection<T>>::onboard(RawOrigin::Signed(who).into()).unwrap();147 }148}149150fn get_licenses<T: Config + configuration::Config>(count: u32) {151 let candidates = (0..count)152 .map(|c| account("candidate", c, SEED))153 .collect::<Vec<_>>();154 assert!(155 <LicenseBond<T>>::get() > 0u32.into(),156 "Bond cannot be zero!"157 );158159 for who in candidates {160 T::Currency::set_balance(&who, <LicenseBond<T>>::get() * 2u32.into());161 <CollatorSelection<T>>::get_license(RawOrigin::Signed(who.clone()).into()).unwrap();162 }163}164165/// `Currency::minimum_balance` was used originally, but in unique-chain, we have166/// zero existential deposit, thus triggering zero bond assertion.167fn balance_unit<T: Config>() -> BalanceOf<T> {168 200u32.into()169}170171/// Our benchmarking environment already has invulnerables registered.172const INITIAL_INVULNERABLES: u32 = 2;173174benchmarks! {175 where_clause { where176 T: pallet_authorship::Config + session::Config + configuration::Config177 }178179 // todo:collator this and all the following do not work for some reason, going all the way up to 10 in length180 // Both invulnerables and candidates count together against MaxCollators.181 // Maybe try putting it in braces? 1 .. (T::MaxCollators::get() - 2)182 add_invulnerable {183 let b in 1 .. T::MaxCollators::get() - INITIAL_INVULNERABLES - 1;184 register_validators::<T>(b);185 register_invulnerables::<T>(b);186187 // log::info!("{} {}", <Invulnerables<T>>::get().len(), b);188189 let new_invulnerable: T::AccountId = whitelisted_caller();190 let bond: BalanceOf<T> = balance_unit::<T>() * 2u32.into();191 T::Currency::set_balance(&new_invulnerable, bond.clone());192193 <session::Pallet<T>>::set_keys(194 RawOrigin::Signed(new_invulnerable.clone()).into(),195 keys::<T>(b + 1),196 Vec::new()197 ).unwrap();198199 let root_origin = T::UpdateOrigin::try_successful_origin().unwrap();200 }: {201 assert_ok!(202 <CollatorSelection<T>>::add_invulnerable(root_origin, new_invulnerable.clone())203 );204 }205 verify {206 assert_last_event::<T>(Event::InvulnerableAdded{invulnerable: new_invulnerable}.into());207 }208209 remove_invulnerable {210 let b in 1 .. T::MaxCollators::get() - INITIAL_INVULNERABLES - 1;211 register_validators::<T>(b);212 register_invulnerables::<T>(b);213214 let root_origin = T::UpdateOrigin::try_successful_origin().unwrap();215 let leaving = <Invulnerables<T>>::get().last().unwrap().clone();216 whitelist!(leaving);217 }: {218 assert_ok!(219 <CollatorSelection<T>>::remove_invulnerable(root_origin, leaving.clone())220 );221 }222 verify {223 assert_last_event::<T>(Event::InvulnerableRemoved{invulnerable: leaving}.into());224 }225226 get_license {227 let c in 1 .. T::MaxCollators::get() - 1;228229 <LicenseBond<T>>::put(balance_unit::<T>());230231 register_validators::<T>(c);232 get_licenses::<T>(c);233234 let caller: T::AccountId = whitelisted_caller();235 let bond: BalanceOf<T> = balance_unit::<T>() * 2u32.into();236 T::Currency::set_balance(&caller, bond.clone());237238 <session::Pallet<T>>::set_keys(239 RawOrigin::Signed(caller.clone()).into(),240 keys::<T>(c + 1),241 Vec::new()242 ).unwrap();243244 }: _(RawOrigin::Signed(caller.clone()))245 verify {246 assert_last_event::<T>(Event::LicenseObtained{account_id: caller, deposit: bond / 2u32.into()}.into());247 }248249 // worst case is when we have all the max-candidate slots filled except one, and we fill that250 // one.251 onboard {252 let c in 1 .. T::MaxCollators::get() - INITIAL_INVULNERABLES - 1;253254 <LicenseBond<T>>::put(balance_unit::<T>());255 <DesiredCollators<T>>::put(c + INITIAL_INVULNERABLES + 1);256257 register_validators::<T>(c);258 register_candidates::<T>(c);259260 let caller: T::AccountId = whitelisted_caller();261 let bond: BalanceOf<T> = balance_unit::<T>() * 2u32.into();262 T::Currency::set_balance(&caller, bond.clone());263264 let origin = RawOrigin::Signed(caller.clone());265266 <session::Pallet<T>>::set_keys(267 origin.clone().into(),268 keys::<T>(c + 1),269 Vec::new()270 ).unwrap();271272 assert_ok!(273 <CollatorSelection<T>>::get_license(origin.clone().into())274 );275 }: _(origin)276 verify {277 assert_last_event::<T>(Event::CandidateAdded{account_id: caller}.into());278 }279280 // worst case is the last candidate leaving.281 offboard {282 let c in 1 .. T::MaxCollators::get();283 <LicenseBond<T>>::put(balance_unit::<T>());284 <DesiredCollators<T>>::put(c + INITIAL_INVULNERABLES);285286 register_validators::<T>(c);287 register_candidates::<T>(c);288289 let leaving = <Candidates<T>>::get().last().unwrap().clone();290 whitelist!(leaving);291 }: _(RawOrigin::Signed(leaving.clone()))292 verify {293 assert_last_event::<T>(Event::CandidateRemoved{account_id: leaving}.into());294 }295296 // worst case is the last candidate leaving.297 release_license {298 let c in 1 .. T::MaxCollators::get();299 let bond = balance_unit::<T>();300 <LicenseBond<T>>::put(bond);301 <DesiredCollators<T>>::put(c + INITIAL_INVULNERABLES);302303 register_validators::<T>(c);304 register_candidates::<T>(c);305306 let leaving = <Candidates<T>>::get().last().unwrap().clone();307 whitelist!(leaving);308 }: _(RawOrigin::Signed(leaving.clone()))309 verify {310 assert_last_event::<T>(Event::LicenseReleased{account_id: leaving, deposit_returned: bond}.into());311 }312313 // worst case is the last candidate leaving.314 force_release_license {315 let c in 1 .. T::MaxCollators::get();316 let bond = balance_unit::<T>();317 <LicenseBond<T>>::put(bond);318 <DesiredCollators<T>>::put(c + INITIAL_INVULNERABLES);319320 register_validators::<T>(c);321 register_candidates::<T>(c);322323 let leaving = <Candidates<T>>::get().last().unwrap().clone();324 whitelist!(leaving);325 let origin = T::UpdateOrigin::try_successful_origin().unwrap();326 }: {327 assert_ok!(328 <CollatorSelection<T>>::force_release_license(origin, leaving.clone())329 );330 }331 verify {332 assert_last_event::<T>(Event::LicenseReleased{account_id: leaving, deposit_returned: bond}.into());333 }334335 // worst case is paying a non-existing candidate account.336 note_author {337 <LicenseBond<T>>::put(balance_unit::<T>());338 T::Currency::set_balance(339 &<CollatorSelection<T>>::account_id(),340 balance_unit::<T>() * 4u32.into(),341 );342 let author = account("author", 0, SEED);343 let new_block: T::BlockNumber = 10u32.into();344345 frame_system::Pallet::<T>::set_block_number(new_block);346 assert!(T::Currency::balance(&author) == 0u32.into());347 }: {348 <CollatorSelection<T> as EventHandler<_, _>>::note_author(author.clone())349 } verify {350 assert!(T::Currency::balance(&author) > 0u32.into());351 assert_eq!(frame_system::Pallet::<T>::block_number(), new_block);352 }353354 // worst case for new session.355 new_session {356 let r in 1 .. T::MaxCollators::get();357 let c in 1 .. T::MaxCollators::get();358359 <LicenseBond<T>>::put(balance_unit::<T>());360 <DesiredCollators<T>>::put(c + INITIAL_INVULNERABLES);361 frame_system::Pallet::<T>::set_block_number(0u32.into());362363 register_validators::<T>(c);364 register_candidates::<T>(c);365366 let new_block: T::BlockNumber = 1800u32.into();367 let zero_block: T::BlockNumber = 0u32.into();368 let candidates = <Candidates<T>>::get();369370 let non_removals = c.saturating_sub(r);371372 for i in 0..c {373 <LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), zero_block);374 }375376 if non_removals > 0 {377 for i in 0..non_removals {378 <LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), new_block);379 }380 } else {381 for i in 0..c {382 <LastAuthoredBlock<T>>::insert(candidates[i as usize].clone(), new_block);383 }384 }385386 let pre_length = <Candidates<T>>::get().len();387388 frame_system::Pallet::<T>::set_block_number(new_block);389390 assert!(<Candidates<T>>::get().len() == c as usize);391 }: {392 <CollatorSelection<T> as SessionManager<_>>::new_session(0)393 } verify {394 if c > r {395 assert!(<Candidates<T>>::get().len() < pre_length);396 } else {397 assert!(<Candidates<T>>::get().len() == pre_length);398 }399 }400}401402impl_benchmark_test_suite!(403 CollatorSelection,404 crate::mock::new_test_ext(),405 crate::mock::Test,406);pallets/collator-selection/src/lib.rsdiffbeforeafterboth--- a/pallets/collator-selection/src/lib.rs
+++ b/pallets/collator-selection/src/lib.rs
@@ -90,6 +90,10 @@
mod benchmarking;
pub mod weights;
+use frame_support::traits::fungible::Inspect;
+
+type BalanceOf<T> =
+ <<T as Config>::Currency as Inspect<<T as frame_system::Config>::AccountId>>::Balance;
#[frame_support::pallet]
pub mod pallet {
use super::*;
@@ -111,11 +115,6 @@
use frame_system::pallet_prelude::*;
use pallet_session::SessionManager;
use sp_runtime::{Perbill, traits::Convert};
- use pallet_configuration::{
- CollatorSelectionDesiredCollatorsOverride as DesiredCollators,
- CollatorSelectionLicenseBondOverride as LicenseBond,
- CollatorSelectionKickThresholdOverride as KickThreshold, BalanceOf,
- };
use sp_staking::SessionIndex;
/// A convertor from collators id. Since this pallet does not have stash/controller, this is
@@ -129,9 +128,12 @@
/// Configure the pallet by specifying the parameters and types on which it depends.
#[pallet::config]
- pub trait Config: frame_system::Config + pallet_configuration::Config {
+ pub trait Config: frame_system::Config {
/// Overarching event type.
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
+ type Currency: Mutate<Self::AccountId>
+ + MutateHold<Self::AccountId>
+ + BalancedHold<Self::AccountId>;
/// Origin that can dictate updating parameters of this pallet.
type UpdateOrigin: EnsureOrigin<Self::RuntimeOrigin>;
@@ -163,7 +165,13 @@
type WeightInfo: WeightInfo;
#[pallet::constant]
- type LicenceBondIdentifier: Get<<<Self as pallet_configuration::Config>::Currency as InspectHold<Self::AccountId>>::Reason>;
+ type LicenceBondIdentifier: Get<<Self::Currency as InspectHold<Self::AccountId>>::Reason>;
+
+ type DesiredCollators: Get<u32>;
+
+ type LicenseBond: Get<BalanceOf<Self>>;
+
+ type KickThreshold: Get<Self::BlockNumber>;
}
#[pallet::pallet]
@@ -365,7 +373,7 @@
Error::<T>::ValidatorNotRegistered
);
- let deposit = <LicenseBond<T>>::get();
+ let deposit = T::LicenseBond::get();
T::Currency::hold(&T::LicenceBondIdentifier::get(), &who, deposit)?;
LicenseDepositOf::<T>::insert(who.clone(), deposit);
@@ -396,7 +404,7 @@
let length = <Candidates<T>>::decode_len().unwrap_or_default()
+ <Invulnerables<T>>::decode_len().unwrap_or_default();
ensure!(
- (length as u32) < <DesiredCollators<T>>::get(),
+ (length as u32) < T::DesiredCollators::get(),
Error::<T>::TooManyCandidates
);
ensure!(
@@ -415,7 +423,7 @@
// First authored block is current block plus kick threshold to handle session delay
<LastAuthoredBlock<T>>::insert(
who.clone(),
- frame_system::Pallet::<T>::block_number() + <KickThreshold<T>>::get(),
+ frame_system::Pallet::<T>::block_number() + T::KickThreshold::get(),
);
Ok(candidates.len())
}
@@ -574,7 +582,7 @@
candidates: BoundedVec<T::AccountId, T::MaxCollators>,
) -> BoundedVec<T::AccountId, T::MaxCollators> {
let now = frame_system::Pallet::<T>::block_number();
- let kick_threshold = <KickThreshold<T>>::get();
+ let kick_threshold = T::KickThreshold::get();
candidates
.into_iter()
.filter_map(|c| {
pallets/collator-selection/src/mock.rsdiffbeforeafterboth--- a/pallets/collator-selection/src/mock.rs
+++ b/pallets/collator-selection/src/mock.rs
@@ -63,7 +63,6 @@
Balances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>},
CollatorSelection: collator_selection::{Pallet, Call, Storage, Event<T>},
Authorship: pallet_authorship::{Pallet, Storage},
- Configuration: pallet_configuration::{Pallet, Call, Storage, Event<T>},
}
);
@@ -216,21 +215,6 @@
pub AppPromotionDailyRate: Perbill = Perbill::from_rational(5u32, 10_000);
pub const DayRelayBlocks: u32 = 1;
pub const LicenceBondIdentifier: [u8; 16] = *b"licenceidentifie";
-}
-
-impl pallet_configuration::Config for Test {
- type RuntimeEvent = RuntimeEvent;
- type Currency = Balances;
- type DefaultCollatorSelectionMaxCollators = MaxCollators;
- type DefaultCollatorSelectionKickThreshold = KickThreshold;
- type DefaultCollatorSelectionLicenseBond = LicenseBond;
- // the following constants we don't care about
- type DefaultWeightToFeeCoefficient = DefaultWeightToFeeCoefficient;
- type DefaultMinGasPrice = DefaultMinGasPrice;
- type MaxXcmAllowedLocations = MaxXcmAllowedLocations;
- type AppPromotionDailyRate = AppPromotionDailyRate;
- type DayRelayBlocks = DayRelayBlocks;
- type WeightInfo = pallet_configuration::weights::SubstrateWeight<Self>;
}
ord_parameter_types! {
@@ -246,11 +230,7 @@
pub struct IsRegistered;
impl ValidatorRegistration<u64> for IsRegistered {
fn is_registered(id: &u64) -> bool {
- if *id == 7u64 {
- false
- } else {
- true
- }
+ *id != 7u64
}
}
@@ -265,6 +245,10 @@
type ValidatorIdOf = IdentityCollator;
type ValidatorRegistration = IsRegistered;
type LicenceBondIdentifier = LicenceBondIdentifier;
+ type Currency = Balances;
+ type DesiredCollators = MaxCollators;
+ type LicenseBond = LicenseBond;
+ type KickThreshold = KickThreshold;
type WeightInfo = ();
}
@@ -277,7 +261,11 @@
let ed = <Test as pallet_balances::Config>::ExistentialDeposit::get();
- let balances = vec![(1, 100), (2, 100), (3, 100), (4, 100), (5, 100), (33, ed)];
+ let balances: Vec<(u64, u64)> = (1..=<Test as Config>::DesiredCollators::get() as u64 + 1)
+ .map(|i| (i, 100))
+ .chain(core::iter::once((33, ed)))
+ .collect();
+
let keys = balances
.iter()
.map(|&(i, _)| {
pallets/collator-selection/src/tests.rsdiffbeforeafterboth--- a/pallets/collator-selection/src/tests.rs
+++ b/pallets/collator-selection/src/tests.rs
@@ -30,19 +30,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-use crate as collator_selection;
+use crate::{self as collator_selection, Config};
use crate::{mock::*, Error};
use frame_support::{
assert_noop, assert_ok,
traits::{fungible, GenesisBuild, OnInitialize},
};
-use frame_system::RawOrigin;
use sp_runtime::{traits::BadOrigin, TokenError};
-use pallet_configuration::{
- CollatorSelectionDesiredCollatorsOverride as DesiredCollators,
- CollatorSelectionKickThresholdOverride as KickThreshold,
- CollatorSelectionLicenseBondOverride as LicenseBond,
-};
use scale_info::prelude::*;
fn get_license_and_onboard(account_id: <Test as frame_system::Config>::AccountId) {
@@ -57,9 +51,6 @@
#[test]
fn basic_setup_works() {
new_test_ext().execute_with(|| {
- assert_eq!(<DesiredCollators<Test>>::get(), 5);
- assert_eq!(<LicenseBond<Test>>::get(), 10);
-
assert!(CollatorSelection::candidates().is_empty());
assert_eq!(CollatorSelection::invulnerables(), vec![1, 2]);
});
@@ -120,51 +111,6 @@
assert_noop!(
CollatorSelection::remove_invulnerable(RuntimeOrigin::signed(RootAccount::get()), 1),
Error::<Test>::TooFewInvulnerables
- );
- });
-}
-
-#[test]
-fn set_desired_collators_works() {
- new_test_ext().execute_with(|| {
- // given
- assert_eq!(<DesiredCollators<Test>>::get(), 5);
-
- // can set
- assert_ok!(Configuration::set_collator_selection_desired_collators(
- RawOrigin::Root.into(),
- Some(7)
- ));
- assert_eq!(<DesiredCollators<Test>>::get(), 7);
-
- // rejects bad origin
- assert_noop!(
- Configuration::set_collator_selection_desired_collators(
- RuntimeOrigin::signed(1),
- Some(8)
- ),
- BadOrigin
- );
- });
-}
-
-#[test]
-fn set_license_bond() {
- new_test_ext().execute_with(|| {
- // given
- assert_eq!(<LicenseBond<Test>>::get(), 10);
-
- // can set
- assert_ok!(Configuration::set_collator_selection_license_bond(
- RawOrigin::Root.into(),
- Some(7)
- ));
- assert_eq!(<LicenseBond<Test>>::get(), 7);
-
- // rejects bad origin.
- assert_noop!(
- Configuration::set_collator_selection_license_bond(RuntimeOrigin::signed(1), Some(8)),
- BadOrigin
);
});
}
@@ -187,26 +133,19 @@
#[test]
fn cannot_onboard_candidate_if_too_many() {
new_test_ext().execute_with(|| {
- // reset desired candidates
- <pallet_configuration::CollatorSelectionDesiredCollatorsOverride<Test>>::put(0);
-
- // can still get a license.
- assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(4)));
-
- // can't accept anyone anymore.
- assert_noop!(
- CollatorSelection::onboard(RuntimeOrigin::signed(4)),
- Error::<Test>::TooManyCandidates,
- );
-
- // reset desired candidates to invulnerables + 1
- <pallet_configuration::CollatorSelectionDesiredCollatorsOverride<Test>>::put(3);
- assert_ok!(CollatorSelection::onboard(RuntimeOrigin::signed(4)));
+ // can accept desired value of collators.
+ for c in 3u64..=(<Test as Config>::DesiredCollators::get()).into() {
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(c)));
+ assert_ok!(CollatorSelection::onboard(RuntimeOrigin::signed(c)));
+ }
// but no more.
- assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(5)));
+ let undesired_collator = (<Test as Config>::DesiredCollators::get() + 1) as u64;
+ assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(
+ undesired_collator
+ )));
assert_noop!(
- CollatorSelection::onboard(RuntimeOrigin::signed(5)),
+ CollatorSelection::onboard(RuntimeOrigin::signed(undesired_collator)),
Error::<Test>::TooManyCandidates,
);
})
@@ -227,8 +166,8 @@
fn cannot_obtain_license_if_poor() {
new_test_ext().execute_with(|| {
let ed = <Test as pallet_balances::Config>::ExistentialDeposit::get();
- assert_eq!(Balances::free_balance(&3), 100);
- assert_eq!(Balances::free_balance(&33), ed);
+ assert_eq!(Balances::free_balance(3), 100);
+ assert_eq!(Balances::free_balance(33), ed);
// works
assert_ok!(CollatorSelection::get_license(RuntimeOrigin::signed(3)));
@@ -266,21 +205,18 @@
#[test]
fn becoming_candidate_works() {
new_test_ext().execute_with(|| {
- // given
- assert_eq!(<DesiredCollators<Test>>::get(), 5);
- assert_eq!(<LicenseBond<Test>>::get(), 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);
+ assert_eq!(Balances::free_balance(3), 100);
+ assert_eq!(Balances::free_balance(4), 100);
get_license_and_onboard(3);
get_license_and_onboard(4);
- assert_eq!(Balances::free_balance(&3), 90);
- assert_eq!(Balances::free_balance(&4), 90);
+ assert_eq!(Balances::free_balance(3), 90);
+ assert_eq!(Balances::free_balance(4), 90);
assert_eq!(CollatorSelection::candidates().len(), 2);
});
@@ -513,7 +449,7 @@
assert_eq!(SessionHandlerCollators::get(), vec![1, 2, 3, 4]);
assert_eq!(CollatorSelection::candidates(), vec![4]);
- assert_eq!(<KickThreshold<Test>>::get(), 10);
+ // assert_eq!(<KickThreshold<Test>>::get(), 10);
assert_eq!(CollatorSelection::last_authored_block(4), 20);
initialize_to_block(30);
pallets/configuration/CHANGELOG.mddiffbeforeafterboth--- a/pallets/configuration/CHANGELOG.md
+++ b/pallets/configuration/CHANGELOG.md
@@ -3,7 +3,12 @@
All notable changes to this project will be documented in this file.
<!-- bureaucrate goes here -->
+## [0.2.0] - 2023-06-27
+
+### Major change
+- Architecture fixed: in the configuration of this pallet, bounds on the associated type were determined not by the functional requirements for this pallet itself, but by the pallet that had tight coupling with it.
+
## [0.1.3] - 2022-12-29
### Added
@@ -20,4 +25,4 @@
### Other changes
-- build: Upgrade polkadot to v0.9.27 2c498572636f2b34d53b1c51b7283a761a7dc90a
\ No newline at end of file
+- build: Upgrade polkadot to v0.9.27 2c498572636f2b34d53b1c51b7283a761a7dc90a
pallets/configuration/Cargo.tomldiffbeforeafterboth--- a/pallets/configuration/Cargo.toml
+++ b/pallets/configuration/Cargo.toml
@@ -1,7 +1,7 @@
[package]
edition = "2021"
name = "pallet-configuration"
-version = "0.1.3"
+version = "0.2.0"
[dependencies]
# Note: `package = "parity-scale-codec"` must be supplied since the `Encode` macro searches for it.
@@ -22,11 +22,11 @@
default = ["std"]
runtime-benchmarks = ["frame-benchmarking"]
std = [
+ "codec/std",
"fp-evm/std",
"frame-benchmarking/std",
"frame-support/std",
"frame-system/std",
- "codec/std",
"sp-arithmetic/std",
"sp-core/std",
"sp-std/std",
pallets/configuration/src/benchmarking.rsdiffbeforeafterboth--- a/pallets/configuration/src/benchmarking.rs
+++ b/pallets/configuration/src/benchmarking.rs
@@ -19,7 +19,7 @@
use super::*;
use frame_benchmarking::benchmarks;
use frame_system::{EventRecord, RawOrigin};
-use frame_support::{assert_ok, traits::fungible::Inspect};
+use frame_support::assert_ok;
fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
let events = frame_system::Pallet::<T>::events();
@@ -30,7 +30,10 @@
}
benchmarks! {
- where_clause { where T: Config }
+ where_clause { where
+ T: Config,
+ T::Balance: From<u32>
+ }
set_weight_to_fee_coefficient_override {
let coeff: u64 = 999;
@@ -60,7 +63,7 @@
let max: u32 = 999;
}: {
assert_ok!(
- <Pallet<T>>::set_collator_selection_desired_collators(RawOrigin::Root.into(), Some(max.clone()))
+ <Pallet<T>>::set_collator_selection_desired_collators(RawOrigin::Root.into(), Some(max))
);
}
verify {
@@ -68,10 +71,10 @@
}
set_collator_selection_license_bond {
- let bond_cost: Option<BalanceOf<T>> = Some(T::Currency::minimum_balance() * 10u32.into());
+ let bond_cost: Option<T::Balance> = Some(1000u32.into());
}: {
assert_ok!(
- <Pallet<T>>::set_collator_selection_license_bond(RawOrigin::Root.into(), bond_cost.clone())
+ <Pallet<T>>::set_collator_selection_license_bond(RawOrigin::Root.into(), bond_cost)
);
}
verify {
@@ -82,7 +85,7 @@
let threshold: Option<T::BlockNumber> = Some(900u32.into());
}: {
assert_ok!(
- <Pallet<T>>::set_collator_selection_kick_threshold(RawOrigin::Root.into(), threshold.clone())
+ <Pallet<T>>::set_collator_selection_kick_threshold(RawOrigin::Root.into(), threshold)
);
}
verify {
pallets/configuration/src/lib.rsdiffbeforeafterboth--- a/pallets/configuration/src/lib.rs
+++ b/pallets/configuration/src/lib.rs
@@ -22,6 +22,7 @@
pallet,
weights::{WeightToFeePolynomial, WeightToFeeCoefficients, WeightToFeeCoefficient, Weight},
traits::Get,
+ Parameter,
};
use codec::{Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;
@@ -42,27 +43,33 @@
mod pallet {
use super::*;
use frame_support::{
- traits::{fungible, Get},
- pallet_prelude::{StorageValue, ValueQuery, DispatchResult, IsType},
+ traits::{Get},
+ pallet_prelude::{
+ StorageValue, ValueQuery, DispatchResult, IsType, Member, MaybeSerializeDeserialize,
+ },
log,
+ dispatch::{Codec, fmt::Debug},
};
- use frame_system::{pallet_prelude::OriginFor, ensure_root, Config as SystemConfig};
-
+ use frame_system::{pallet_prelude::OriginFor, ensure_root};
+ use sp_arithmetic::{FixedPointOperand, traits::AtLeast32BitUnsigned};
pub use crate::weights::WeightInfo;
- pub type BalanceOf<T> =
- <<T as Config>::Currency as fungible::Inspect<<T as SystemConfig>::AccountId>>::Balance;
#[pallet::config]
pub trait Config: frame_system::Config {
/// Overarching event type.
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
- type Currency: fungible::Inspect<Self::AccountId>
- + fungible::Mutate<Self::AccountId>
- + fungible::MutateFreeze<Self::AccountId>
- + fungible::InspectHold<Self::AccountId>
- + fungible::MutateHold<Self::AccountId>
- + fungible::BalancedHold<Self::AccountId>;
+ type Balance: Parameter
+ + Member
+ + AtLeast32BitUnsigned
+ + Codec
+ + Default
+ + Copy
+ + MaybeSerializeDeserialize
+ + Debug
+ + MaxEncodedLen
+ + TypeInfo
+ + FixedPointOperand;
#[pallet::constant]
type DefaultWeightToFeeCoefficient: Get<u64>;
@@ -79,7 +86,7 @@
#[pallet::constant]
type DefaultCollatorSelectionMaxCollators: Get<u32>;
#[pallet::constant]
- type DefaultCollatorSelectionLicenseBond: Get<BalanceOf<Self>>;
+ type DefaultCollatorSelectionLicenseBond: Get<Self::Balance>;
#[pallet::constant]
type DefaultCollatorSelectionKickThreshold: Get<Self::BlockNumber>;
@@ -94,7 +101,7 @@
desired_collators: Option<u32>,
},
NewCollatorLicenseBond {
- bond_cost: Option<BalanceOf<T>>,
+ bond_cost: Option<T::Balance>,
},
NewCollatorKickThreshold {
length_in_blocks: Option<T::BlockNumber>,
@@ -130,7 +137,7 @@
#[pallet::storage]
pub type CollatorSelectionLicenseBondOverride<T: Config> = StorageValue<
- Value = BalanceOf<T>,
+ Value = T::Balance,
QueryKind = ValueQuery,
OnEmpty = T::DefaultCollatorSelectionLicenseBond,
>;
@@ -221,7 +228,7 @@
#[pallet::weight(T::WeightInfo::set_collator_selection_license_bond())]
pub fn set_collator_selection_license_bond(
origin: OriginFor<T>,
- amount: Option<BalanceOf<T>>,
+ amount: Option<<T as Config>::Balance>,
) -> DispatchResult {
ensure_root(origin)?;
if let Some(amount) = amount {
runtime/common/config/pallets/collator_selection.rsdiffbeforeafterboth--- a/runtime/common/config/pallets/collator_selection.rs
+++ b/runtime/common/config/pallets/collator_selection.rs
@@ -23,7 +23,10 @@
};
use sp_runtime::Perbill;
use up_common::constants::{UNIQUE, MILLIUNIQUE};
-
+use pallet_configuration::{
+ CollatorSelectionKickThresholdOverride, CollatorSelectionLicenseBondOverride,
+ CollatorSelectionDesiredCollatorsOverride,
+};
parameter_types! {
pub const SessionOffset: BlockNumber = 0;
}
@@ -60,6 +63,9 @@
pub const MaxAdditionalFields: u32 = 100;
pub const MaxRegistrars: u32 = 20;
pub const LicenceBondIdentifier: [u8; 16] = *b"licenceidentifie";
+ pub LicenseBond: Balance = CollatorSelectionLicenseBondOverride::<Runtime>::get();
+ pub DesiredCollators: u32 = CollatorSelectionDesiredCollatorsOverride::<Runtime>::get();
+ pub KickThreshold: BlockNumber = CollatorSelectionKickThresholdOverride::<Runtime>::get();
}
impl pallet_identity::Config for Runtime {
@@ -84,6 +90,7 @@
impl pallet_collator_selection::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
+ type Currency = Balances;
// We allow root only to execute privileged collator selection operations.
type UpdateOrigin = EnsureRoot<AccountId>;
type TreasuryAccountId = TreasuryAccountId;
@@ -95,4 +102,7 @@
type ValidatorRegistration = Session;
type WeightInfo = pallet_collator_selection::weights::SubstrateWeight<Runtime>;
type LicenceBondIdentifier = LicenceBondIdentifier;
+ type DesiredCollators = DesiredCollators;
+ type LicenseBond = LicenseBond;
+ type KickThreshold = KickThreshold;
}
runtime/common/config/pallets/mod.rsdiffbeforeafterboth--- a/runtime/common/config/pallets/mod.rs
+++ b/runtime/common/config/pallets/mod.rs
@@ -129,7 +129,7 @@
impl pallet_configuration::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
- type Currency = Balances;
+ type Balance = Balance;
type DefaultWeightToFeeCoefficient = ConstU64<{ up_common::constants::WEIGHT_TO_FEE_COEFF }>;
type DefaultMinGasPrice = ConstU64<{ up_common::constants::MIN_GAS_PRICE }>;
type DefaultCollatorSelectionMaxCollators = MaxCollators;
tests/src/benchmarks/mintFee/index.tsdiffbeforeafterboth--- a/tests/src/benchmarks/mintFee/index.ts
+++ b/tests/src/benchmarks/mintFee/index.ts
@@ -360,7 +360,7 @@
susbstrateReceiver.addressRaw,
PROPERTIES.slice(0, setup.propertiesNumber),
)
- .send({from: ethSigner, gas: 25_000_000});
+ .send({from: ethSigner});
},
);