From e459cda0e1eb7250c9e3869810a731a43f111ea9 Mon Sep 17 00:00:00 2001 From: NachoPal Date: Mon, 7 Aug 2023 16:29:15 +0200 Subject: [PATCH 1/8] update to polkadot-v1.0.0 --- Cargo.toml | 20 ++++++++++---------- docs/im-online-integration.md | 4 ++-- readme.md | 4 ++-- src/benchmarking.rs | 1 - src/lib.rs | 14 +++++++------- src/mock.rs | 22 ++++++++++------------ 6 files changed, 31 insertions(+), 34 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 28f7eca..bda8989 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'substrate-validator-set' -version = '0.9.42' +version = '1.0.0' authors = ['Gautam Dhameja '] edition = '2021' license = 'Apache-2.0' @@ -13,27 +13,27 @@ version = '1.0.126' [dependencies.sp-core] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.sp-io] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.sp-runtime] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.sp-std] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.sp-staking] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.log] default-features = false @@ -49,23 +49,23 @@ version = '3.0.0' default-features = false git = 'https://github.com/paritytech/substrate.git' optional = true -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.frame-support] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.frame-system] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.pallet-session] default-features = false features = ['historical'] git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' [dependencies.scale-info] default-features = false diff --git a/docs/im-online-integration.md b/docs/im-online-integration.md index 70c52fb..eb6c2a9 100644 --- a/docs/im-online-integration.md +++ b/docs/im-online-integration.md @@ -12,7 +12,7 @@ [dependencies.pallet-im-online] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' ``` ```toml @@ -150,7 +150,7 @@ construct_runtime!( [dependencies.pallet-im-online] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' ``` * Import `ImOnlineId` in the `chain_spec.rs`. diff --git a/readme.md b/readme.md index 99eb417..cd1bc7e 100644 --- a/readme.md +++ b/readme.md @@ -2,7 +2,7 @@ A [Substrate](https://github.com/paritytech/substrate/) pallet to add/remove authorities/validators in PoA networks. -**Note: Current master is compatible with Substrate [polkadot-v0.9.43](https://github.com/paritytech/substrate/tree/polkadot-v0.9.43) branch. For older versions, please see releases/tags.** +**Note: Current master is compatible with Substrate [polkadot-v1.0.0](https://github.com/paritytech/substrate/tree/polkadot-v1.0.0) branch. For older versions, please see releases/tags.** ## Setup with Substrate Node Template @@ -22,7 +22,7 @@ version = '0.9.42' [dependencies.pallet-session] default-features = false git = 'https://github.com/paritytech/substrate.git' -branch = 'polkadot-v0.9.43' +branch = 'polkadot-v1.0.0' ``` ```toml diff --git a/src/benchmarking.rs b/src/benchmarking.rs index b4c4f8a..5d33d09 100644 --- a/src/benchmarking.rs +++ b/src/benchmarking.rs @@ -1,7 +1,6 @@ #![cfg(feature = "runtime-benchmarks")] use super::*; -use crate::Pallet as ValidatorSet; use frame_benchmarking::v1::{account, benchmarks, BenchmarkError}; use frame_support::traits::EnsureOrigin; diff --git a/src/lib.rs b/src/lib.rs index f12f9a9..a75eeb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,7 @@ mod mock; mod tests; pub mod weights; +use frame_system::pallet_prelude::*; use frame_support::{ ensure, pallet_prelude::*, @@ -35,7 +36,6 @@ pub const LOG_TARGET: &'static str = "runtime::validator-set"; #[frame_support::pallet()] pub mod pallet { use super::*; - use frame_system::pallet_prelude::*; /// Configure the pallet by specifying the parameters and types on which it /// depends. @@ -102,7 +102,7 @@ pub mod pallet { } #[pallet::genesis_build] - impl GenesisBuild for GenesisConfig { + impl BuildGenesisConfig for GenesisConfig { fn build(&self) { Pallet::::initialize_validators(&self.initial_validators); } @@ -233,20 +233,20 @@ impl pallet_session::SessionManager for Pallet { fn start_session(_start_index: u32) {} } -impl EstimateNextSessionRotation for Pallet { - fn average_session_length() -> T::BlockNumber { +impl EstimateNextSessionRotation> for Pallet { + fn average_session_length() -> BlockNumberFor { Zero::zero() } fn estimate_current_session_progress( - _now: T::BlockNumber, + _now: BlockNumberFor, ) -> (Option, frame_support::dispatch::Weight) { (None, Zero::zero()) } fn estimate_next_session_rotation( - _now: T::BlockNumber, - ) -> (Option, frame_support::dispatch::Weight) { + _now: BlockNumberFor, + ) -> (Option>, frame_support::dispatch::Weight) { (None, Zero::zero()) } } diff --git a/src/mock.rs b/src/mock.rs index f05c6cd..e8a3a4c 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -4,15 +4,15 @@ use super::*; use crate as validator_set; -use frame_support::{parameter_types, traits::GenesisBuild, BasicExternalities}; +use frame_support::{parameter_types, BasicExternalities}; use frame_system::EnsureRoot; use pallet_session::*; use sp_core::{crypto::key_types::DUMMY, H256}; use sp_runtime::{ impl_opaque_keys, - testing::{Header, UintAuthorityId}, + testing::{UintAuthorityId}, traits::{BlakeTwo256, IdentityLookup, OpaqueKeys}, - KeyTypeId, RuntimeAppPublic, + BuildStorage, KeyTypeId, RuntimeAppPublic, }; use std::cell::RefCell; @@ -53,15 +53,14 @@ impl OpaqueKeys for PreUpgradeMockSessionKeys { } } -type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; frame_support::construct_runtime!( pub struct Test - where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic, + // where + // Block = Block, + // NodeBlock = Block, + // UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system, ValidatorSet: validator_set, @@ -124,7 +123,7 @@ pub fn authorities() -> Vec { } pub fn new_test_ext() -> sp_io::TestExternalities { - let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); + let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); let keys: Vec<_> = NEXT_VALIDATORS .with(|l| l.borrow().iter().cloned().map(|i| (i, i, UintAuthorityId(i).into())).collect()); BasicExternalities::execute_with_storage(&mut t, || { @@ -157,14 +156,13 @@ impl frame_system::Config for Test { type BlockLength = (); type DbWeight = (); type RuntimeOrigin = RuntimeOrigin; - type Index = u64; - type BlockNumber = u64; + type Nonce = u64; type RuntimeCall = RuntimeCall; type Hash = H256; type Hashing = BlakeTwo256; type AccountId = u64; type Lookup = IdentityLookup; - type Header = Header; + type Block = Block; type RuntimeEvent = RuntimeEvent; type BlockHashCount = BlockHashCount; type Version = (); From 33cff3651333c33531f7a5b94f06821e878e58de Mon Sep 17 00:00:00 2001 From: NachoPal Date: Mon, 7 Aug 2023 16:30:36 +0200 Subject: [PATCH 2/8] update to polkadot-v1.0.0 + --- src/mock.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/mock.rs b/src/mock.rs index e8a3a4c..de0b7b4 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -57,10 +57,6 @@ type Block = frame_system::mocking::MockBlock; frame_support::construct_runtime!( pub struct Test - // where - // Block = Block, - // NodeBlock = Block, - // UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system, ValidatorSet: validator_set, From 19b3d3d5f13095a26ce269c057070468c856050a Mon Sep 17 00:00:00 2001 From: NachoPal Date: Mon, 7 Aug 2023 16:35:21 +0200 Subject: [PATCH 3/8] update to polkadot-v1.0.0 ++ --- src/mock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mock.rs b/src/mock.rs index de0b7b4..c0b94a5 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -10,7 +10,7 @@ use pallet_session::*; use sp_core::{crypto::key_types::DUMMY, H256}; use sp_runtime::{ impl_opaque_keys, - testing::{UintAuthorityId}, + testing::UintAuthorityId, traits::{BlakeTwo256, IdentityLookup, OpaqueKeys}, BuildStorage, KeyTypeId, RuntimeAppPublic, }; From d2bf9b2ad21ea486f03edce56b1abddd6df24a4c Mon Sep 17 00:00:00 2001 From: NachoPal Date: Mon, 7 Aug 2023 16:36:58 +0200 Subject: [PATCH 4/8] update to polkadot-v1.0.0 +++ --- src/mock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mock.rs b/src/mock.rs index c0b94a5..334fa48 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -158,7 +158,7 @@ impl frame_system::Config for Test { type Hashing = BlakeTwo256; type AccountId = u64; type Lookup = IdentityLookup; - type Block = Block; + type Block = Block; type RuntimeEvent = RuntimeEvent; type BlockHashCount = BlockHashCount; type Version = (); From 8bef892faa0c68860c8d94409c5eced4ac2e87d6 Mon Sep 17 00:00:00 2001 From: NachoPal Date: Mon, 7 Aug 2023 17:06:15 +0200 Subject: [PATCH 5/8] impl Default for GenesisConfig --- src/lib.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a75eeb0..a29bc2d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,6 +23,7 @@ use frame_support::{ ensure, pallet_prelude::*, traits::{EstimateNextSessionRotation, Get, ValidatorSet, ValidatorSetWithIdentification}, + DefaultNoBound, }; use log; pub use pallet::*; @@ -90,17 +91,11 @@ pub mod pallet { impl Hooks> for Pallet {} #[pallet::genesis_config] + #[derive(DefaultNoBound)] pub struct GenesisConfig { pub initial_validators: Vec, } - #[cfg(feature = "std")] - impl Default for GenesisConfig { - fn default() -> Self { - Self { initial_validators: Default::default() } - } - } - #[pallet::genesis_build] impl BuildGenesisConfig for GenesisConfig { fn build(&self) { From 554cb948d1ecc4b313f925e8fc38b5856108dd95 Mon Sep 17 00:00:00 2001 From: NachoPal Date: Thu, 10 Aug 2023 15:51:11 +0200 Subject: [PATCH 6/8] added OnOffenceHandler --- src/lib.rs | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a29bc2d..161c56f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,8 +27,13 @@ use frame_support::{ }; use log; pub use pallet::*; -use sp_runtime::traits::{Convert, Zero}; -use sp_staking::offence::{Offence, OffenceError, ReportOffence}; +use sp_runtime::{traits::{Convert, Zero}, Perbill,}; +use sp_staking::{ + offence::{ + DisableStrategy, Offence, OffenceError, OffenceDetails, OnOffenceHandler, ReportOffence, + }, + SessionIndex, +}; use sp_std::prelude::*; pub use weights::*; @@ -207,6 +212,36 @@ impl Pallet { // Clear the offline validator list to avoid repeated deletion. >::put(Vec::::new()); } + + // Removes disabled validators from the validator set. + // It is called in the session change hook and removes the validators + // who were disabled. We do not check for `MinAuthorities` here, + // because the offline validators will not produce blocks and will + // have the same overall effect on the runtime. + fn remove_disabled_validators() { + let validators = >::validators(); + let validators_index_to_remove = >::disabled_validators(); + let mut validators_to_remove: Vec = Default::default(); + + validators_index_to_remove.iter().for_each(|i| { + if let Some(validator) = validators.get(*i as usize) { + validators_to_remove.push(validator.clone()); + } + }); + + // Delete from active validator set. + >::mutate(|vs| vs.retain(|v| !validators_to_remove.contains(v))); + log::debug!( + target: LOG_TARGET, + "Initiated removal of {:?} disabled validators.", + validators_to_remove.len() + ); + } + + // Disable validator from current `pallet_session` validators set + fn disable_validator(validator: &T::ValidatorId) -> bool { + >::disable(validator) + } } // Provides the new set of validators to the session module when session is @@ -217,6 +252,9 @@ impl pallet_session::SessionManager for Pallet { // Remove any offline validators. This will only work when the runtime // also has the im-online pallet. Self::remove_offline_validators(); + // Remove any disabled validators. This will only work when the runtime + // also has the offences and session::historical pallets + Self::remove_disabled_validators(); log::debug!(target: LOG_TARGET, "New session called; updated validator set provided."); @@ -260,7 +298,7 @@ impl ValidatorSet for Pallet { type ValidatorId = T::ValidatorId; type ValidatorIdOf = ValidatorOf; - fn session_index() -> sp_staking::SessionIndex { + fn session_index() -> SessionIndex { pallet_session::Pallet::::current_index() } @@ -296,3 +334,40 @@ impl> false } } + +// Implementation of OnOffenceHandler. +// This is for the Offences + Historical pallets integration. +impl + OnOffenceHandler, Weight> + for Pallet +where + T: pallet_session::historical::Config, +{ + fn on_offence( + offenders: &[OffenceDetails< + T::AccountId, + pallet_session::historical::IdentificationTuple, + >], + _slash_fraction: &[Perbill], + _slash_session: SessionIndex, + _disable_strategy: DisableStrategy, + ) -> Weight { + let mut consumed_weight = Weight::from_parts(0, 0); + let mut add_db_reads_writes = |reads, writes| { + consumed_weight += T::DbWeight::get().reads_writes(reads, writes); + }; + + offenders.iter().for_each(|o| { + let offender = o.offender.clone(); + if Self::disable_validator(&offender.0) { + // Validator was not yet disabled, it is added to `DisabledValidators` + add_db_reads_writes(1, 1); + } else { + // Validator was already disabled, it is not added to `DisabledValidators` (no writes) + add_db_reads_writes(1, 0); + } + }); + + consumed_weight + } +} From 6f70f451c491c46a9802e2834f7f29d5eb21d7a3 Mon Sep 17 00:00:00 2001 From: NachoPal Date: Fri, 11 Aug 2023 11:48:05 +0200 Subject: [PATCH 7/8] added OnDisabled trait --- src/lib.rs | 70 ++++++++++++++++++++++++++++++++++++++++++----------- src/mock.rs | 1 + 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 161c56f..54d2694 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -39,6 +39,35 @@ pub use weights::*; pub const LOG_TARGET: &'static str = "runtime::validator-set"; +/// Trait that defines an action to be executed when a validator is disabled +/// It is agnostic about what is done in that action, `on_disabled` method just +/// expect a `Weight` in return +pub trait OnDisabled +where + T: frame_system::Config + pallet_session::Config +{ + fn on_disabled( + offender: &T::ValidatorId, + slash_fraction: &[Perbill], + slash_session: SessionIndex, + disable_strategy: DisableStrategy, + ) -> Weight; +} + +impl OnDisabled for () +where + T: frame_system::Config + pallet_session::Config +{ + fn on_disabled( + _offenders: &T::ValidatorId, + _slash_fraction: &[Perbill], + _slash_session: SessionIndex, + _disable_strategy: DisableStrategy, + ) -> Weight { + Weight::zero() + } +} + #[frame_support::pallet()] pub mod pallet { use super::*; @@ -57,6 +86,9 @@ pub mod pallet { /// auto removal. type MinAuthorities: Get; + /// Action to be executed when a validator is disabled + type OnDisabled: OnDisabled; + /// Information on runtime weights. type WeightInfo: WeightInfo; } @@ -335,7 +367,7 @@ impl> } } -// Implementation of OnOffenceHandler. +// Implementation of `OnOffenceHandler`. // This is for the Offences + Historical pallets integration. impl OnOffenceHandler, Weight> @@ -348,23 +380,33 @@ where T::AccountId, pallet_session::historical::IdentificationTuple, >], - _slash_fraction: &[Perbill], - _slash_session: SessionIndex, - _disable_strategy: DisableStrategy, + slash_fraction: &[Perbill], + slash_session: SessionIndex, + disable_strategy: DisableStrategy, ) -> Weight { - let mut consumed_weight = Weight::from_parts(0, 0); - let mut add_db_reads_writes = |reads, writes| { - consumed_weight += T::DbWeight::get().reads_writes(reads, writes); - }; + let mut consumed_weight = Weight::zero(); offenders.iter().for_each(|o| { let offender = o.offender.clone(); - if Self::disable_validator(&offender.0) { - // Validator was not yet disabled, it is added to `DisabledValidators` - add_db_reads_writes(1, 1); - } else { - // Validator was already disabled, it is not added to `DisabledValidators` (no writes) - add_db_reads_writes(1, 0); + + match disable_strategy { + DisableStrategy::WhenSlashed | DisableStrategy::Always => { + if Self::disable_validator(&offender.0) { + // Validator was not yet disabled, it is added to `DisabledValidators` + consumed_weight += T::DbWeight::get().reads_writes(1, 1); + // Execute `on_disabled` action + consumed_weight += T::OnDisabled::on_disabled( + &offender.0, + slash_fraction, + slash_session, + disable_strategy + ); + } else { + // Validator was already disabled, it is not added to `DisabledValidators` (no writes) + consumed_weight += T::DbWeight::get().reads_writes(1, 0); + } + }, + DisableStrategy::Never => {}, } }); diff --git a/src/mock.rs b/src/mock.rs index 334fa48..b81bd1d 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -180,6 +180,7 @@ impl validator_set::Config for Test { type AddRemoveOrigin = EnsureRoot; type RuntimeEvent = RuntimeEvent; type MinAuthorities = MinAuthorities; + type OnDisabled = (); type WeightInfo = (); } From bf519b89732fcb4c2c2481cf76784be1162efafe Mon Sep 17 00:00:00 2001 From: NachoPal Date: Fri, 11 Aug 2023 13:08:32 +0200 Subject: [PATCH 8/8] add local DisabledValidators + MinAuthoritiesOnDisabled --- src/lib.rs | 101 +++++++++++++++++++++++++++++++++++++--------------- src/mock.rs | 2 ++ 2 files changed, 75 insertions(+), 28 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 54d2694..cbdde87 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,7 +27,7 @@ use frame_support::{ }; use log; pub use pallet::*; -use sp_runtime::{traits::{Convert, Zero}, Perbill,}; +use sp_runtime::{traits::{Convert, Zero}, Perbill, RuntimeDebug}; use sp_staking::{ offence::{ DisableStrategy, Offence, OffenceError, OffenceDetails, OnOffenceHandler, ReportOffence, @@ -39,9 +39,9 @@ pub use weights::*; pub const LOG_TARGET: &'static str = "runtime::validator-set"; -/// Trait that defines an action to be executed when a validator is disabled +/// Trait that defines an action to be executed when a validator is disabled. /// It is agnostic about what is done in that action, `on_disabled` method just -/// expect a `Weight` in return +/// expects a `Weight` in return. pub trait OnDisabled where T: frame_system::Config + pallet_session::Config @@ -68,6 +68,15 @@ where } } +/// Reason for a validator to be removed from the active set +#[derive(RuntimeDebug)] +pub enum RemovalReason { + /// The validator went offline + Offline, + /// The validator was disabled + Disabled, +} + #[frame_support::pallet()] pub mod pallet { use super::*; @@ -89,6 +98,9 @@ pub mod pallet { /// Action to be executed when a validator is disabled type OnDisabled: OnDisabled; + /// Check `MinAuthorities` before removing validators when disabled + type MinAuthoritiesOnDisabled: Get; + /// Information on runtime weights. type WeightInfo: WeightInfo; } @@ -105,6 +117,10 @@ pub mod pallet { #[pallet::getter(fn offline_validators)] pub type OfflineValidators = StorageValue<_, Vec, ValueQuery>; + #[pallet::storage] + #[pallet::getter(fn disabled_validators)] + pub type DisabledValidators = StorageValue<_, Vec, ValueQuery>; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { @@ -220,11 +236,17 @@ impl Pallet { } // Adds offline validators to a local cache for removal on new session. - fn mark_for_removal(validator_id: T::ValidatorId) { + fn mark_offline_for_removal(validator_id: T::ValidatorId) { >::mutate(|v| v.push(validator_id)); log::debug!(target: LOG_TARGET, "Offline validator marked for auto removal."); } + // Adds disabled validators to a local cache for removal on new session. + fn mark_disabled_for_removal(validator_id: T::ValidatorId) { + >::mutate(|v| v.push(validator_id)); + log::debug!(target: LOG_TARGET, "Disabled validator marked for auto removal."); + } + // Removes offline validators from the validator set and clears the offline // cache. It is called in the session change hook and removes the validators // who were reported offline during the session that is ending. We do not @@ -233,41 +255,60 @@ impl Pallet { fn remove_offline_validators() { let validators_to_remove = >::get(); - // Delete from active validator set. - >::mutate(|vs| vs.retain(|v| !validators_to_remove.contains(v))); - log::debug!( - target: LOG_TARGET, - "Initiated removal of {:?} offline validators.", - validators_to_remove.len() - ); + // Validators will be always removed because there is not any kid of checking + let _ = Self::do_remove_validators(&validators_to_remove, false, RemovalReason::Offline); - // Clear the offline validator list to avoid repeated deletion. + // Clear the offline validator list to avoid repeated deletion. >::put(Vec::::new()); } // Removes disabled validators from the validator set. // It is called in the session change hook and removes the validators - // who were disabled. We do not check for `MinAuthorities` here, - // because the offline validators will not produce blocks and will - // have the same overall effect on the runtime. + // who were disabled. fn remove_disabled_validators() { - let validators = >::validators(); - let validators_index_to_remove = >::disabled_validators(); - let mut validators_to_remove: Vec = Default::default(); + let validators_to_remove = >::get(); + + match Self::do_remove_validators(&validators_to_remove, T::MinAuthoritiesOnDisabled::get(), RemovalReason::Disabled) { + Ok(_) => { + // Clear the offline validator list to avoid repeated deletion. + >::put(Vec::::new()); + }, + Err(_) => { + // Number of active validators was going to drop under `MinAuthorities` + log::error!( + target: LOG_TARGET, + "Number of validators was going to drop below MinAuthorities ({:?}) after removing {:?} disabled validators", + T::MinAuthorities::get(), + validators_to_remove.len(), + ); + } + } + } - validators_index_to_remove.iter().for_each(|i| { - if let Some(validator) = validators.get(*i as usize) { - validators_to_remove.push(validator.clone()); + fn do_remove_validators( + validators: &Vec, + check_min_authorities: bool, + reason: RemovalReason + ) -> Result<(), DispatchError> { + let validators_len_to_remove = validators.len(); + let current_validators_len = Self::validators().len(); + + if check_min_authorities { + if let Some(validators_left_len) = current_validators_len.checked_sub(validators_len_to_remove) { + ensure!(validators_left_len as u32 >= T::MinAuthorities::get(), Error::::TooLowValidatorCount); } - }); + } + + >::mutate(|vs| vs.retain(|v| !validators.contains(v))); - // Delete from active validator set. - >::mutate(|vs| vs.retain(|v| !validators_to_remove.contains(v))); log::debug!( target: LOG_TARGET, - "Initiated removal of {:?} disabled validators.", - validators_to_remove.len() + "Initiated removal of {:?} validators, reason: {:?}.", + validators.len(), + reason ); + + Ok(()) } // Disable validator from current `pallet_session` validators set @@ -353,7 +394,7 @@ impl> let offenders = offence.offenders(); for (v, _) in offenders.into_iter() { - Self::mark_for_removal(v); + Self::mark_offline_for_removal(v); } Ok(()) @@ -392,8 +433,12 @@ where match disable_strategy { DisableStrategy::WhenSlashed | DisableStrategy::Always => { if Self::disable_validator(&offender.0) { - // Validator was not yet disabled, it is added to `DisabledValidators` + // Validator was not yet disabled, it is added to pallet_session `DisabledValidators` consumed_weight += T::DbWeight::get().reads_writes(1, 1); + // Validator is added to local `DisabledValidators` + Self::mark_disabled_for_removal(offender.0.clone()); + consumed_weight += T::DbWeight::get().reads_writes(1, 1); + // Execute `on_disabled` action consumed_weight += T::OnDisabled::on_disabled( &offender.0, diff --git a/src/mock.rs b/src/mock.rs index b81bd1d..1791484 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -174,6 +174,7 @@ impl frame_system::Config for Test { parameter_types! { pub const MinAuthorities: u32 = 2; + pub const MinAuthoritiesOnDisabled: bool = true; } impl validator_set::Config for Test { @@ -181,6 +182,7 @@ impl validator_set::Config for Test { type RuntimeEvent = RuntimeEvent; type MinAuthorities = MinAuthorities; type OnDisabled = (); + type MinAuthoritiesOnDisabled = MinAuthoritiesOnDisabled; type WeightInfo = (); }