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..cbdde87 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,24 +18,68 @@ mod mock; mod tests; pub mod weights; +use frame_system::pallet_prelude::*; use frame_support::{ ensure, pallet_prelude::*, traits::{EstimateNextSessionRotation, Get, ValidatorSet, ValidatorSetWithIdentification}, + DefaultNoBound, }; 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, RuntimeDebug}; +use sp_staking::{ + offence::{ + DisableStrategy, Offence, OffenceError, OffenceDetails, OnOffenceHandler, ReportOffence, + }, + SessionIndex, +}; use sp_std::prelude::*; 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 +/// expects 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() + } +} + +/// 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::*; - use frame_system::pallet_prelude::*; /// Configure the pallet by specifying the parameters and types on which it /// depends. @@ -51,6 +95,12 @@ pub mod pallet { /// auto removal. type MinAuthorities: Get; + /// 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; } @@ -67,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 { @@ -90,19 +144,13 @@ 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 GenesisBuild for GenesisConfig { + impl BuildGenesisConfig for GenesisConfig { fn build(&self) { Pallet::::initialize_validators(&self.initial_validators); } @@ -188,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 @@ -201,17 +255,66 @@ 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))); + // 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. + >::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. + fn remove_disabled_validators() { + 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(), + ); + } + } + } + + 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))); + log::debug!( target: LOG_TARGET, - "Initiated removal of {:?} offline validators.", - validators_to_remove.len() + "Initiated removal of {:?} validators, reason: {:?}.", + validators.len(), + reason ); - // Clear the offline validator list to avoid repeated deletion. - >::put(Vec::::new()); - } + Ok(()) + } + + // 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 @@ -222,6 +325,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."); @@ -233,20 +339,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()) } } @@ -265,7 +371,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() } @@ -288,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(()) @@ -301,3 +407,54 @@ 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::zero(); + + offenders.iter().for_each(|o| { + let offender = o.offender.clone(); + + match disable_strategy { + DisableStrategy::WhenSlashed | DisableStrategy::Always => { + if Self::disable_validator(&offender.0) { + // 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, + 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 => {}, + } + }); + + consumed_weight + } +} diff --git a/src/mock.rs b/src/mock.rs index f05c6cd..1791484 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,10 @@ 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, { System: frame_system, ValidatorSet: validator_set, @@ -124,7 +119,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 +152,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 = (); @@ -180,12 +174,15 @@ impl frame_system::Config for Test { parameter_types! { pub const MinAuthorities: u32 = 2; + pub const MinAuthoritiesOnDisabled: bool = true; } impl validator_set::Config for Test { type AddRemoveOrigin = EnsureRoot; type RuntimeEvent = RuntimeEvent; type MinAuthorities = MinAuthorities; + type OnDisabled = (); + type MinAuthoritiesOnDisabled = MinAuthoritiesOnDisabled; type WeightInfo = (); }