diff --git a/Cargo.lock b/Cargo.lock index 60d51800f2f73..5271021e06a08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1828,7 +1828,7 @@ dependencies = [ "polkadot-service 0.2.2", "polkadot-transaction-pool 0.1.0", "slog 2.2.3 (registry+https://github.com/rust-lang/crates.io-index)", - "substrate-cli 0.2.2", + "substrate-cli 0.2.4", "substrate-client 0.1.0", "substrate-codec 0.1.0", "substrate-extrinsic-pool 0.1.0", @@ -2614,7 +2614,7 @@ dependencies = [ [[package]] name = "substrate-cli" -version = "0.2.2" +version = "0.2.4" dependencies = [ "ansi_term 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)", "app_dirs 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/demo/runtime/src/lib.rs b/demo/runtime/src/lib.rs index ebc7c777da0c6..6ee577c8170eb 100644 --- a/demo/runtime/src/lib.rs +++ b/demo/runtime/src/lib.rs @@ -121,7 +121,7 @@ impl Convert for SessionKeyConversion { } impl session::Trait for Concrete { - const NOTE_OFFLINE_POSITION: u32 = 1; + const NOTE_MISSED_PROPOSAL_POSITION: u32 = 1; type ConvertAccountIdToSessionKey = SessionKeyConversion; type OnSessionChange = Staking; } diff --git a/polkadot/consensus/src/offline_tracker.rs b/polkadot/consensus/src/offline_tracker.rs index efb317ea5c913..eb6c2480c414a 100644 --- a/polkadot/consensus/src/offline_tracker.rs +++ b/polkadot/consensus/src/offline_tracker.rs @@ -21,14 +21,18 @@ use polkadot_primitives::AccountId; use std::collections::HashMap; use std::time::{Instant, Duration}; -// time before we report a validator. -const REPORT_TIME: Duration = Duration::from_secs(60 * 5); - struct Observed { last_round_end: Instant, offline_since: Instant, } +#[derive(Eq, PartialEq)] +enum Activity { + Offline, + StillOffline(Duration), + Online, +} + impl Observed { fn new() -> Observed { let now = Instant::now(); @@ -38,31 +42,32 @@ impl Observed { } } - fn note_round_end(&mut self, was_online: bool) { - let now = Instant::now(); - + fn note_round_end(&mut self, now: Instant, was_online: Option) { self.last_round_end = now; - if was_online { + if let Some(false) = was_online { self.offline_since = now; } } - fn is_active(&self) -> bool { + /// Returns what we have observed about the online/offline state of the validator. + fn activity(&self) -> Activity { // can happen if clocks are not monotonic - if self.offline_since > self.last_round_end { return true } - self.last_round_end.duration_since(self.offline_since) < REPORT_TIME + if self.offline_since > self.last_round_end { return Activity::Online } + if self.offline_since == self.last_round_end { return Activity::Offline } + Activity::StillOffline(self.last_round_end.duration_since(self.offline_since)) } } /// Tracks offline validators and can issue a report for those offline. pub struct OfflineTracker { observed: HashMap, + block_instant: Instant, } impl OfflineTracker { /// Create a new tracker. pub fn new() -> Self { - OfflineTracker { observed: HashMap::new() } + OfflineTracker { observed: HashMap::new(), block_instant: Instant::now() } } /// Note new consensus is starting with the given set of validators. @@ -71,23 +76,33 @@ impl OfflineTracker { let set: HashSet<_> = validators.iter().cloned().collect(); self.observed.retain(|k, _| set.contains(k)); + + self.block_instant = Instant::now(); } /// Note that a round has ended. pub fn note_round_end(&mut self, validator: AccountId, was_online: bool) { - self.observed.entry(validator) - .or_insert_with(Observed::new) - .note_round_end(was_online); + self.observed.entry(validator).or_insert_with(Observed::new); + for (val, obs) in self.observed.iter_mut() { + obs.note_round_end( + self.block_instant, + if val == &validator { + Some(was_online) + } else { + None + } + ) + } } /// Generate a vector of indices for offline account IDs. pub fn reports(&self, validators: &[AccountId]) -> Vec { validators.iter() .enumerate() - .filter_map(|(i, v)| if self.is_online(v) { - None - } else { + .filter_map(|(i, v)| if self.is_known_offline_now(v) { Some(i as u32) + } else { + None }) .collect() } @@ -101,13 +116,15 @@ impl OfflineTracker { }; // we must think all validators reported externally are offline. - let thinks_online = self.is_online(v); - !thinks_online + self.is_known_offline_now(v) }) } - fn is_online(&self, v: &AccountId) -> bool { - self.observed.get(v).map(Observed::is_active).unwrap_or(true) + /// Rwturns true only if we have seen the validator miss the last round. For further + /// rounds where we can't say for sure that they're still offline, we give them the + /// benefit of the doubt. + fn is_known_offline_now(&self, v: &AccountId) -> bool { + self.observed.get(v).map(|o| o.activity() == Activity::Offline).unwrap_or(false) } } @@ -121,17 +138,30 @@ mod tests { let v = [0; 32].into(); let v2 = [1; 32].into(); let v3 = [2; 32].into(); + tracker.note_new_block(&[v, v2, v3]); tracker.note_round_end(v, true); tracker.note_round_end(v2, true); tracker.note_round_end(v3, true); + assert_eq!(tracker.reports(&[v, v2, v3]), vec![0u32; 0]); + + tracker.note_new_block(&[v, v2, v3]); + tracker.note_round_end(v, true); + tracker.note_round_end(v2, false); + tracker.note_round_end(v3, true); + assert_eq!(tracker.reports(&[v, v2, v3]), vec![1]); - let slash_time = REPORT_TIME + Duration::from_secs(5); - tracker.observed.get_mut(&v).unwrap().offline_since -= slash_time; - tracker.observed.get_mut(&v2).unwrap().offline_since -= slash_time; + tracker.note_new_block(&[v, v2, v3]); + tracker.note_round_end(v, false); + assert_eq!(tracker.reports(&[v, v2, v3]), vec![0]); - assert_eq!(tracker.reports(&[v, v2, v3]), vec![0, 1]); + tracker.note_new_block(&[v, v2, v3]); + tracker.note_round_end(v, false); + tracker.note_round_end(v2, true); + tracker.note_round_end(v3, false); + assert_eq!(tracker.reports(&[v, v2, v3]), vec![0, 2]); - tracker.note_new_block(&[v, v3]); + tracker.note_new_block(&[v, v2]); + tracker.note_round_end(v, false); assert_eq!(tracker.reports(&[v, v2, v3]), vec![0]); } } diff --git a/polkadot/runtime/src/checked_block.rs b/polkadot/runtime/src/checked_block.rs index d193d26963fd0..6ed0cee17614f 100644 --- a/polkadot/runtime/src/checked_block.rs +++ b/polkadot/runtime/src/checked_block.rs @@ -16,10 +16,10 @@ //! Typesafe block interaction. -use super::{Call, Block, TIMESTAMP_SET_POSITION, PARACHAINS_SET_POSITION, NOTE_OFFLINE_POSITION}; +use super::{Call, Block, TIMESTAMP_SET_POSITION, PARACHAINS_SET_POSITION, NOTE_MISSED_PROPOSAL_POSITION}; use timestamp::Call as TimestampCall; use parachains::Call as ParachainsCall; -use session::Call as SessionCall; +use staking::Call as StakingCall; use primitives::parachain::CandidateReceipt; /// Provides a type-safe wrapper around a structurally valid block. @@ -92,8 +92,8 @@ impl CheckedBlock { /// Extract the noted offline validator indices (if any) from the block. pub fn noted_offline(&self) -> &[u32] { - self.inner.extrinsics.get(NOTE_OFFLINE_POSITION as usize).and_then(|xt| match xt.extrinsic.function { - Call::Session(SessionCall::note_offline(ref x)) => Some(&x[..]), + self.inner.extrinsics.get(NOTE_MISSED_PROPOSAL_POSITION as usize).and_then(|xt| match xt.extrinsic.function { + Call::Staking(StakingCall::note_missed_proposal(ref x)) => Some(&x[..]), _ => None, }).unwrap_or(&[]) } diff --git a/polkadot/runtime/src/lib.rs b/polkadot/runtime/src/lib.rs index dfe8e61048986..1762b78044dde 100644 --- a/polkadot/runtime/src/lib.rs +++ b/polkadot/runtime/src/lib.rs @@ -86,7 +86,7 @@ pub const TIMESTAMP_SET_POSITION: u32 = 0; /// The position of the parachains set extrinsic. pub const PARACHAINS_SET_POSITION: u32 = 1; /// The position of the offline nodes noting extrinsic. -pub const NOTE_OFFLINE_POSITION: u32 = 2; +pub const NOTE_MISSED_PROPOSAL_POSITION: u32 = 2; /// The address format for describing accounts. pub type Address = staking::Address; @@ -111,8 +111,8 @@ pub struct Concrete; pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: ver_str!("polkadot"), impl_name: ver_str!("parity-polkadot"), - authoring_version: 1, - spec_version: 3, + authoring_version: 2, + spec_version: 4, impl_version: 0, }; @@ -162,7 +162,7 @@ impl Convert for SessionKeyConversion { } impl session::Trait for Concrete { - const NOTE_OFFLINE_POSITION: u32 = NOTE_OFFLINE_POSITION; + const NOTE_MISSED_PROPOSAL_POSITION: u32 = NOTE_MISSED_PROPOSAL_POSITION; type ConvertAccountIdToSessionKey = SessionKeyConversion; type OnSessionChange = Staking; } diff --git a/polkadot/runtime/src/parachains.rs b/polkadot/runtime/src/parachains.rs index 16cdcdca33249..8fdb2d01a7e58 100644 --- a/polkadot/runtime/src/parachains.rs +++ b/polkadot/runtime/src/parachains.rs @@ -265,7 +265,7 @@ mod tests { type Header = Header; } impl session::Trait for Test { - const NOTE_OFFLINE_POSITION: u32 = 1; + const NOTE_MISSED_PROPOSAL_POSITION: u32 = 1; type ConvertAccountIdToSessionKey = Identity; type OnSessionChange = (); } diff --git a/polkadot/runtime/src/utils.rs b/polkadot/runtime/src/utils.rs index acef06092539f..d3bfe7449448f 100644 --- a/polkadot/runtime/src/utils.rs +++ b/polkadot/runtime/src/utils.rs @@ -21,7 +21,7 @@ use super::{Call, UncheckedExtrinsic, Extrinsic, Staking}; use runtime_primitives::traits::{Checkable, AuxLookup}; use timestamp::Call as TimestampCall; use parachains::Call as ParachainsCall; -use session::Call as SessionCall; +use staking::Call as StakingCall; /// Produces the list of inherent extrinsics. pub fn inherent_extrinsics(data: ::primitives::InherentData) -> Vec { @@ -41,7 +41,7 @@ pub fn inherent_extrinsics(data: ::primitives::InherentData) -> Vec; } diff --git a/substrate/runtime/democracy/src/lib.rs b/substrate/runtime/democracy/src/lib.rs index 85b2a505227b1..8a794a75470bc 100644 --- a/substrate/runtime/democracy/src/lib.rs +++ b/substrate/runtime/democracy/src/lib.rs @@ -391,7 +391,7 @@ mod tests { type Header = Header; } impl session::Trait for Test { - const NOTE_OFFLINE_POSITION: u32 = 1; + const NOTE_MISSED_PROPOSAL_POSITION: u32 = 1; type ConvertAccountIdToSessionKey = Identity; type OnSessionChange = staking::Module; } diff --git a/substrate/runtime/executive/src/lib.rs b/substrate/runtime/executive/src/lib.rs index a918f062e86a5..f380acc0574bb 100644 --- a/substrate/runtime/executive/src/lib.rs +++ b/substrate/runtime/executive/src/lib.rs @@ -252,7 +252,7 @@ mod tests { type Header = Header; } impl session::Trait for Test { - const NOTE_OFFLINE_POSITION: u32 = 1; + const NOTE_MISSED_PROPOSAL_POSITION: u32 = 1; type ConvertAccountIdToSessionKey = Identity; type OnSessionChange = staking::Module; } diff --git a/substrate/runtime/primitives/src/traits.rs b/substrate/runtime/primitives/src/traits.rs index 3c89087d70dc3..7b9fe8568e3d4 100644 --- a/substrate/runtime/primitives/src/traits.rs +++ b/substrate/runtime/primitives/src/traits.rs @@ -26,7 +26,8 @@ use codec::{Codec, Encode}; pub use integer_sqrt::IntegerSquareRoot; pub use num_traits::{Zero, One, Bounded}; pub use num_traits::ops::checked::{CheckedAdd, CheckedSub, CheckedMul, CheckedDiv}; -use rstd::ops::{Add, Sub, Mul, Div, Rem, AddAssign, SubAssign, MulAssign, DivAssign, RemAssign}; +use rstd::ops::{Add, Sub, Mul, Div, Rem, AddAssign, SubAssign, MulAssign, DivAssign, + RemAssign, Shl, Shr}; /// A lazy value. pub trait Lazy { @@ -132,6 +133,7 @@ pub trait SimpleArithmetic: Mul + MulAssign + Div + DivAssign + Rem + RemAssign + + Shl + Shr + CheckedAdd + CheckedSub + CheckedMul + @@ -145,6 +147,7 @@ impl + MulAssign + Div + DivAssign + Rem + RemAssign + + Shl + Shr + CheckedAdd + CheckedSub + CheckedMul + diff --git a/substrate/runtime/session/src/lib.rs b/substrate/runtime/session/src/lib.rs index 06368d651e2b1..72dc7857c3969 100644 --- a/substrate/runtime/session/src/lib.rs +++ b/substrate/runtime/session/src/lib.rs @@ -51,21 +51,21 @@ use runtime_support::{StorageValue, StorageMap}; use runtime_support::dispatch::Result; /// A session has changed. -pub trait OnSessionChange { +pub trait OnSessionChange { /// Session has changed. - fn on_session_change(time_elapsed: T, bad_validators: Vec); + fn on_session_change(time_elapsed: T, should_reward: bool); } -impl OnSessionChange for () { - fn on_session_change(_: T, _: Vec) {} +impl OnSessionChange for () { + fn on_session_change(_: T, _: bool) {} } pub trait Trait: timestamp::Trait { // the position of the required timestamp-set extrinsic. - const NOTE_OFFLINE_POSITION: u32; + const NOTE_MISSED_PROPOSAL_POSITION: u32; type ConvertAccountIdToSessionKey: Convert; - type OnSessionChange: OnSessionChange; + type OnSessionChange: OnSessionChange; } decl_module! { @@ -80,7 +80,7 @@ decl_module! { #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub enum PrivCall { fn set_length(new: T::BlockNumber) -> Result = 0; - fn force_new_session(normal_rotation: bool) -> Result = 1; + fn force_new_session(apply_rewards: bool) -> Result = 1; } } @@ -139,8 +139,8 @@ impl Module { } /// Forces a new session. - pub fn force_new_session(normal_rotation: bool) -> Result { - >::put(normal_rotation); + pub fn force_new_session(apply_rewards: bool) -> Result { + >::put(apply_rewards); Ok(()) } @@ -148,9 +148,9 @@ impl Module { pub fn note_offline(aux: &T::PublicAux, offline_val_indices: Vec) -> Result { assert!(aux.is_empty()); assert!( - >::extrinsic_index() == T::NOTE_OFFLINE_POSITION, + >::extrinsic_index() == T::NOTE_MISSED_PROPOSAL_POSITION, "note_offline extrinsic must be at position {} in the block", - T::NOTE_OFFLINE_POSITION + T::NOTE_MISSED_PROPOSAL_POSITION ); let vs = Self::validators(); @@ -178,15 +178,15 @@ impl Module { // check block number and call next_session if necessary. let block_number = >::block_number(); let is_final_block = ((block_number - Self::last_length_change()) % Self::length()).is_zero(); - let bad_validators = >::take().unwrap_or_default(); - let should_end_session = >::take().is_some() || !bad_validators.is_empty() || is_final_block; + let (should_end_session, apply_rewards) = >::take() + .map_or((is_final_block, is_final_block), |apply_rewards| (true, apply_rewards)); if should_end_session { - Self::rotate_session(is_final_block, bad_validators); + Self::rotate_session(is_final_block, apply_rewards); } } /// Move onto next session: register the new authority set. - pub fn rotate_session(is_final_block: bool, bad_validators: Vec) { + pub fn rotate_session(is_final_block: bool, apply_rewards: bool) { let now = >::get(); let time_elapsed = now.clone() - Self::current_start(); @@ -206,7 +206,7 @@ impl Module { >::put(block_number); } - T::OnSessionChange::on_session_change(time_elapsed, bad_validators); + T::OnSessionChange::on_session_change(time_elapsed, apply_rewards); // Update any changes in session keys. Self::validators().iter().enumerate().for_each(|(i, v)| { @@ -321,7 +321,7 @@ mod tests { type Moment = u64; } impl Trait for Test { - const NOTE_OFFLINE_POSITION: u32 = 1; + const NOTE_MISSED_PROPOSAL_POSITION: u32 = 1; type ConvertAccountIdToSessionKey = Identity; type OnSessionChange = (); } @@ -357,36 +357,6 @@ mod tests { }); } - #[test] - fn should_identify_broken_validation() { - with_externalities(&mut new_test_ext(), || { - System::set_block_number(2); - assert_eq!(Session::blocks_remaining(), 0); - Timestamp::set_timestamp(0); - assert_ok!(Session::set_length(3)); - Session::check_rotate_session(); - assert_eq!(Session::current_index(), 1); - assert_eq!(Session::length(), 3); - assert_eq!(Session::current_start(), 0); - assert_eq!(Session::ideal_session_duration(), 15); - // ideal end = 0 + 15 * 3 = 15 - // broken_limit = 15 * 130 / 100 = 19 - - System::set_block_number(3); - assert_eq!(Session::blocks_remaining(), 2); - Timestamp::set_timestamp(9); // earliest end = 9 + 2 * 5 = 19; OK. - assert!(!Session::broken_validation()); - Session::check_rotate_session(); - - System::set_block_number(4); - assert_eq!(Session::blocks_remaining(), 1); - Timestamp::set_timestamp(15); // another 1 second late. earliest end = 15 + 1 * 5 = 20; broken. - assert!(Session::broken_validation()); - Session::check_rotate_session(); - assert_eq!(Session::current_index(), 2); - }); - } - #[test] fn should_work_with_early_exit() { with_externalities(&mut new_test_ext(), || { diff --git a/substrate/runtime/staking/src/genesis_config.rs b/substrate/runtime/staking/src/genesis_config.rs index 2987725b7a235..86b6e00240406 100644 --- a/substrate/runtime/staking/src/genesis_config.rs +++ b/substrate/runtime/staking/src/genesis_config.rs @@ -26,7 +26,8 @@ use {runtime_io, primitives}; use super::{Trait, ENUM_SET_SIZE, EnumSet, NextEnumSet, Intentions, CurrentEra, BondingDuration, CreationFee, TransferFee, ReclaimRebate, ExistentialDeposit, TransactionByteFee, TransactionBaseFee, TotalStake, - SessionsPerEra, ValidatorCount, FreeBalance, SessionReward, EarlyEraSlash}; + SessionsPerEra, ValidatorCount, FreeBalance, SessionReward, EarlyEraSlash, + OfflineSlashGrace}; #[derive(Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -46,6 +47,7 @@ pub struct GenesisConfig { pub existential_deposit: T::Balance, pub session_reward: T::Balance, pub early_era_slash: T::Balance, + pub offline_slash_grace: u32, } impl GenesisConfig where T::AccountId: From { @@ -65,6 +67,7 @@ impl GenesisConfig where T::AccountId: From { reclaim_rebate: T::Balance::sa(0), session_reward: T::Balance::sa(0), early_era_slash: T::Balance::sa(0), + offline_slash_grace: 1, } } @@ -92,6 +95,7 @@ impl GenesisConfig where T::AccountId: From { reclaim_rebate: T::Balance::sa(0), session_reward: T::Balance::sa(0), early_era_slash: T::Balance::sa(0), + offline_slash_grace: 1, } } } @@ -113,6 +117,7 @@ impl Default for GenesisConfig { reclaim_rebate: T::Balance::sa(0), session_reward: T::Balance::sa(0), early_era_slash: T::Balance::sa(0), + offline_slash_grace: 0, } } } @@ -136,6 +141,7 @@ impl primitives::BuildStorage for GenesisConfig { Self::hash(>::key()).to_vec() => self.current_era.encode(), Self::hash(>::key()).to_vec() => self.session_reward.encode(), Self::hash(>::key()).to_vec() => self.early_era_slash.encode(), + Self::hash(>::key()).to_vec() => self.offline_slash_grace.encode(), Self::hash(>::key()).to_vec() => total_stake.encode() ]; diff --git a/substrate/runtime/staking/src/lib.rs b/substrate/runtime/staking/src/lib.rs index 825a09f171413..6f51faddf0e44 100644 --- a/substrate/runtime/staking/src/lib.rs +++ b/substrate/runtime/staking/src/lib.rs @@ -52,7 +52,7 @@ use runtime_support::{StorageValue, StorageMap, Parameter}; use runtime_support::dispatch::Result; use session::OnSessionChange; use primitives::traits::{Zero, One, Bounded, RefInto, SimpleArithmetic, Executable, MakePayment, - As, AuxLookup, Member, CheckedAdd, CheckedSub}; + As, AuxLookup, Member, CheckedAdd, CheckedSub, MaybeEmpty}; use address::Address as RawAddress; mod mock; @@ -98,7 +98,40 @@ impl OnAccountKill for () { fn on_account_kill(_who: &AccountId) {} } +/// Preference of what happens on a slash event. +#[cfg_attr(feature = "std", derive(Debug, Serialize, Deserialize))] +#[derive(Eq, PartialEq, Clone, Copy)] +pub struct SlashPreference { + /// Validator should ensure this many more slashes than is necessary before being unstaked. + pub unstake_threshold: u32, +} + +impl Decode for SlashPreference { + fn decode(input: &mut I) -> Option { + Some(SlashPreference { + unstake_threshold: Decode::decode(input)? + }) + } +} + +impl Encode for SlashPreference { + fn encode_to(&self, dest: &mut T) { + self.unstake_threshold.encode_to(dest) + } +} + +impl Default for SlashPreference { + fn default() -> Self { + SlashPreference { + unstake_threshold: 3, + } + } +} + pub trait Trait: system::Trait + session::Trait { + /// The allowed extrinsic position for `missed_proposal` inherent. +// const NOTE_MISSED_PROPOSAL_POSITION: u32; // TODO: uncomment when removed from session::Trait + /// The balance of an account. type Balance: Parameter + SimpleArithmetic + Codec + Default + Copy + As + As + As; /// Type used for storing an account's index; implies the maximum number of accounts the system @@ -117,9 +150,11 @@ decl_module! { pub enum Call where aux: T::PublicAux { fn transfer(aux, dest: RawAddress, value: T::Balance) -> Result = 0; fn stake(aux) -> Result = 1; - fn unstake(aux, index: u32) -> Result = 2; + fn unstake(aux, intentions_index: u32) -> Result = 2; fn nominate(aux, target: RawAddress) -> Result = 3; fn unnominate(aux, target_index: u32) -> Result = 4; + fn register_slash_preference(aux, intentions_index: u32, p: SlashPreference) -> Result = 5; + fn note_missed_proposal(aux, offline_val_indices: Vec) -> Result = 6; } #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] @@ -127,7 +162,8 @@ decl_module! { fn set_sessions_per_era(new: T::BlockNumber) -> Result = 0; fn set_bonding_duration(new: T::BlockNumber) -> Result = 1; fn set_validator_count(new: u32) -> Result = 2; - fn force_new_era(should_slash: bool) -> Result = 3; + fn force_new_era(apply_rewards: bool) -> Result = 3; + fn set_offline_slash_grace(new: u32) -> Result = 4; } } @@ -159,9 +195,13 @@ decl_storage! { pub SessionReward get(session_reward): b"sta:session_reward" => required T::Balance; // Slash, per validator that is taken per abnormal era end. pub EarlyEraSlash get(early_era_slash): b"sta:early_era_slash" => required T::Balance; + // Number of instances of offline reports before slashing begins for validators. + pub OfflineSlashGrace get(offline_slash_grace): b"sta:offline_slash_grace" => default u32; // The current era index. pub CurrentEra get(current_era): b"sta:era" => required T::BlockNumber; + // Preference over how many times the validator should get slashed for being offline before they are automatically unstaked. + pub SlashPreferenceOf get(slash_preference_of): b"sta:slash_preference_of" => default map [ T::AccountId => SlashPreference ]; // All the accounts with a desire to stake. pub Intentions get(intentions): b"sta:wil:" => default Vec; // All nominator -> nominee relationships. @@ -177,8 +217,8 @@ decl_storage! { // The current era stake threshold pub StakeThreshold get(stake_threshold): b"sta:stake_threshold" => required T::Balance; - // The current bad validator slash. - pub CurrentSlash get(current_slash): b"sta:current_slash" => default T::Balance; + // The number of times a given validator has been reported offline. This gets decremented by one each era that passes. + pub SlashCount get(slash_count): b"sta:slash_count" => default map [ T::AccountId => u32 ]; // The next free enumeration set. pub NextEnumSet get(next_enum_set): b"sta:next_enum" => required T::AccountIndex; @@ -247,6 +287,20 @@ impl Module { Self::free_balance(who) + Self::reserved_balance(who) } + /// Balance of a (potential) validator that includes all nominators. + pub fn nomination_balance(who: &T::AccountId) -> T::Balance { + Self::nominators_for(who).iter() + .map(Self::voting_balance) + .fold(Zero::zero(), |acc, x| acc + x) + } + + /// The total balance that can be slashed from an account. + pub fn slashable_balance(who: &T::AccountId) -> T::Balance { + Self::nominators_for(who).iter() + .map(Self::voting_balance) + .fold(Self::voting_balance(who), |acc, x| acc + x) + } + /// Some result as `slash(who, value)` (but without the side-effects) assuming there are no /// balance changes in the meantime and only the reserved balance is not taken into account. pub fn can_slash(who: &T::AccountId, value: T::Balance) -> bool { @@ -263,6 +317,22 @@ impl Module { } } + /// Lookup an T::AccountIndex to get an Id, if there's one there. + pub fn lookup_index(index: T::AccountIndex) -> Option { + let enum_set_size = Self::enum_set_size(); + let set = Self::enum_set(index / enum_set_size); + let i: usize = (index % enum_set_size).as_(); + set.get(i).map(|x| x.clone()) + } + + /// `true` if the account `index` is ready for reclaim. + pub fn can_reclaim(try_index: T::AccountIndex) -> bool { + let enum_set_size = Self::enum_set_size(); + let try_set = Self::enum_set(try_index / enum_set_size); + let i = (try_index % enum_set_size).as_(); + i < try_set.len() && Self::voting_balance(&try_set[i]).is_zero() + } + /// The block at which the `who`'s funds become entirely liquid. pub fn unlock_block(who: &T::AccountId) -> LockStatus { match Self::bondage(who) { @@ -327,18 +397,8 @@ impl Module { /// Retract the desire to stake for the transactor. /// /// Effects will be felt at the beginning of the next era. - fn unstake(aux: &T::PublicAux, position: u32) -> Result { - let aux = aux.ref_into(); - let position = position as usize; - let mut intentions = >::get(); -// let position = intentions.iter().position(|t| t == aux.ref_into()).ok_or("Cannot unstake if not already staked.")?; - if intentions.get(position) != Some(aux) { - return Err("Invalid index") - } - intentions.swap_remove(position); - >::put(intentions); - >::insert(aux.ref_into(), Self::current_era() + Self::bonding_duration()); - Ok(()) + fn unstake(aux: &T::PublicAux, intentions_index: u32) -> Result { + Self::apply_unstake(aux.ref_into(), intentions_index as usize) } fn nominate(aux: &T::PublicAux, target: RawAddress) -> Result { @@ -389,6 +449,64 @@ impl Module { Ok(()) } + /// Set the given account's preference for slashing behaviour should they be a validator. + /// + /// An error (no-op) if `Self::intentions()[intentions_index] != aux`. + fn register_slash_preference( + aux: &T::PublicAux, + intentions_index: u32, + p: SlashPreference + ) -> Result { + let aux = aux.ref_into(); + + if Self::intentions().get(intentions_index as usize) != Some(aux) { + return Err("Invalid index") + } + + >::insert(aux, p); + + Ok(()) + } + + /// Note the previous block's validator missed their opportunity to propose a block. This only comes in + /// if 2/3+1 of the validators agree that no proposal was submitted. It's only relevant + /// for the previous block. + fn note_missed_proposal(aux: &T::PublicAux, offline_val_indices: Vec) -> Result { + assert!(aux.is_empty()); + assert!( + >::extrinsic_index() == T::NOTE_MISSED_PROPOSAL_POSITION, + "note_missed_proposal extrinsic must be at position {} in the block", + T::NOTE_MISSED_PROPOSAL_POSITION + ); + + for validator_index in offline_val_indices.into_iter() { + let v = >::validators()[validator_index as usize].clone(); + let slash_count = Self::slash_count(&v); + >::insert(v.clone(), slash_count + 1); + let grace = Self::offline_slash_grace(); + + if slash_count >= grace { + let instances = slash_count - grace; + let slash = Self::early_era_slash() << instances; + let next_slash = slash << 1u32; + let _ = Self::slash_validator(&v, slash); + if instances >= Self::slash_preference_of(&v).unstake_threshold + || Self::slashable_balance(&v) < next_slash + { + if let Some(pos) = Self::intentions().into_iter().position(|x| &x == &v) { + Self::apply_unstake(&v, pos) + .expect("pos derived correctly from Self::intentions(); \ + apply_unstake can only fail if pos wrong; \ + Self::intentions() doesn't change; qed"); + } + let _ = Self::force_new_era(false); + } + } + } + + Ok(()) + } + // PRIV DISPATCH /// Set the number of sessions in an era. @@ -411,9 +529,15 @@ impl Module { /// Force there to be a new era. This also forces a new session immediately after by /// setting `normal_rotation` to be false. Validators will get slashed. - fn force_new_era(should_slash: bool) -> Result { + fn force_new_era(apply_rewards: bool) -> Result { >::put(()); - >::force_new_session(!should_slash) + >::force_new_session(apply_rewards) + } + + /// Set the offline slash grace period. + fn set_offline_slash_grace(new: u32) -> Result { + >::put(&new); + Ok(()) } // PUBLIC MUTABLES (DANGEROUS) @@ -590,62 +714,76 @@ impl Module { } } - /// Session has just changed. We need to determine whether we pay a reward, slash and/or - /// move to a new era. - fn new_session(actual_elapsed: T::Moment, bad_validators: Vec) { - let session_index = >::current_index(); - let early_exit_era = !bad_validators.is_empty(); - - if early_exit_era { - // slash - let slash = Self::current_slash() + Self::early_era_slash(); - >::put(&slash); - for v in bad_validators.into_iter() { - if let Some(rem) = Self::slash(&v, slash) { - let noms = Self::current_nominators_for(&v); - let total = noms.iter().map(Self::voting_balance).fold(T::Balance::zero(), |acc, x| acc + x); - if !total.is_zero() { - let safe_mul_rational = |b| b * rem / total;// TODO: avoid overflow - for n in noms.iter() { - let _ = Self::slash(n, safe_mul_rational(Self::voting_balance(n))); // best effort - not much that can be done on fail. - } - } + /// Slash a given validator by a specific amount. Removes the slash from their balance by preference, + /// and reduces the nominators' balance if needed. + fn slash_validator(v: &T::AccountId, slash: T::Balance) { + if let Some(rem) = Self::slash(v, slash) { + let noms = Self::current_nominators_for(v); + let total = noms.iter().map(Self::voting_balance).fold(T::Balance::zero(), |acc, x| acc + x); + if !total.is_zero() { + let safe_mul_rational = |b| b * rem / total;// TODO: avoid overflow + for n in noms.iter() { + let _ = Self::slash(n, safe_mul_rational(Self::voting_balance(n))); // best effort - not much that can be done on fail. } } - } else { - // Zero any cumulative slash since we're healthy now. - >::kill(); + } + } - // reward - let ideal_elapsed = >::ideal_session_duration(); - let per65536: u64 = (T::Moment::sa(65536u64) * ideal_elapsed.clone() / actual_elapsed.max(ideal_elapsed)).as_(); - let reward = Self::session_reward() * T::Balance::sa(per65536) / T::Balance::sa(65536u64); + /// Reward a given validator by a specific amount. Add the reward to their, and their nominators' + /// balance, pro-rata. + fn reward_validator(who: &T::AccountId, reward: T::Balance) { + let noms = Self::current_nominators_for(who); + let total = noms.iter().map(Self::voting_balance).fold(Self::voting_balance(who), |acc, x| acc + x); + if !total.is_zero() { + let safe_mul_rational = |b| b * reward / total;// TODO: avoid overflow + for n in noms.iter() { + let _ = Self::reward(n, safe_mul_rational(Self::voting_balance(n))); + } + let _ = Self::reward(who, safe_mul_rational(Self::voting_balance(who))); + } + } + + /// Actually carry out the unstake operation. + /// Assumes `intentions()[intentions_index] == who`. + fn apply_unstake(who: &T::AccountId, intentions_index: usize) -> Result { + let mut intentions = Self::intentions(); + if intentions.get(intentions_index) != Some(who) { + return Err("Invalid index"); + } + intentions.swap_remove(intentions_index); + >::put(intentions); + >::remove(who); + >::remove(who); + >::insert(who, Self::current_era() + Self::bonding_duration()); + Ok(()) + } + + /// Get the reward for the session, assuming it ends with this block. + fn this_session_reward(actual_elapsed: T::Moment) -> T::Balance { + let ideal_elapsed = >::ideal_session_duration(); + let per65536: u64 = (T::Moment::sa(65536u64) * ideal_elapsed.clone() / actual_elapsed.max(ideal_elapsed)).as_(); + Self::session_reward() * T::Balance::sa(per65536) / T::Balance::sa(65536u64) + } + + /// Session has just changed. We need to determine whether we pay a reward, slash and/or + /// move to a new era. + fn new_session(actual_elapsed: T::Moment, should_reward: bool) { + if should_reward { // apply good session reward + let reward = Self::this_session_reward(actual_elapsed); for v in >::validators().iter() { - let noms = Self::current_nominators_for(v); - let total = noms.iter().map(Self::voting_balance).fold(Self::voting_balance(v), |acc, x| acc + x); - if !total.is_zero() { - let safe_mul_rational = |b| b * reward / total;// TODO: avoid overflow - for n in noms.iter() { - let _ = Self::reward(n, safe_mul_rational(Self::voting_balance(n))); - } - let _ = Self::reward(v, safe_mul_rational(Self::voting_balance(v))); - } + Self::reward_validator(v, reward); } } + + let session_index = >::current_index(); if >::take().is_some() || ((session_index - Self::last_era_length_change()) % Self::sessions_per_era()).is_zero() - || early_exit_era { Self::new_era(); } } - /// Balance of a (potential) validator that includes all nominators. - fn nomination_balance(who: &T::AccountId) -> T::Balance { - Self::nominators_for(who).iter().map(Self::voting_balance).fold(Zero::zero(), |acc, x| acc + x) - } - /// The era has changed - enact new staking set. /// /// NOTE: This always happens immediately before a session change to ensure that new validators @@ -662,8 +800,6 @@ impl Module { } } - let minimum_allowed = Self::early_era_slash(); - // evaluate desired staking amounts and nominations and optimise to find the best // combination of validators, then use session::internal::set_validators(). // for now, this just orders would-be stakers by their balances and chooses the top-most @@ -671,8 +807,7 @@ impl Module { // TODO: this is not sound. this should be moved to an off-chain solution mechanism. let mut intentions = >::get() .into_iter() - .map(|v| (Self::voting_balance(&v) + Self::nomination_balance(&v), v)) - .filter(|&(b, _)| b >= minimum_allowed) + .map(|v| (Self::slashable_balance(&v), v)) .collect::>(); intentions.sort_unstable_by(|&(ref b1, _), &(ref b2, _)| b2.cmp(&b1)); @@ -688,6 +823,10 @@ impl Module { .collect::>(); for v in >::validators().iter() { >::remove(v); + let slash_count = >::take(v); + if slash_count > 1 { + >::insert(v, slash_count - 1); + } } for v in vals.iter() { >::insert(v, Self::nominators_for(v)); @@ -699,22 +838,6 @@ impl Module { T::AccountIndex::sa(ENUM_SET_SIZE) } - /// Lookup an T::AccountIndex to get an Id, if there's one there. - pub fn lookup_index(index: T::AccountIndex) -> Option { - let enum_set_size = Self::enum_set_size(); - let set = Self::enum_set(index / enum_set_size); - let i: usize = (index % enum_set_size).as_(); - set.get(i).map(|x| x.clone()) - } - - /// `true` if the account `index` is ready for reclaim. - pub fn can_reclaim(try_index: T::AccountIndex) -> bool { - let enum_set_size = Self::enum_set_size(); - let try_set = Self::enum_set(try_index / enum_set_size); - let i = (try_index % enum_set_size).as_(); - i < try_set.len() && Self::voting_balance(&try_set[i]).is_zero() - } - /// Register a new account (with existential balance). fn new_account(who: &T::AccountId, balance: T::Balance) -> NewAccountOutcome { let enum_set_size = Self::enum_set_size(); @@ -808,9 +931,9 @@ impl Executable for Module { } } -impl OnSessionChange for Module { - fn on_session_change(elapsed: T::Moment, bad_validators: Vec) { - Self::new_session(elapsed, bad_validators); +impl OnSessionChange for Module { + fn on_session_change(elapsed: T::Moment, should_reward: bool) { + Self::new_session(elapsed, should_reward); } } diff --git a/substrate/runtime/staking/src/mock.rs b/substrate/runtime/staking/src/mock.rs index dcc96b03735e1..eef8de0abf630 100644 --- a/substrate/runtime/staking/src/mock.rs +++ b/substrate/runtime/staking/src/mock.rs @@ -45,7 +45,7 @@ impl system::Trait for Test { type Header = Header; } impl session::Trait for Test { - const NOTE_OFFLINE_POSITION: u32 = 1; + const NOTE_MISSED_PROPOSAL_POSITION: u32 = 0; type ConvertAccountIdToSessionKey = Identity; type OnSessionChange = Staking; } @@ -98,6 +98,7 @@ pub fn new_test_ext(ext_deposit: u64, session_length: u64, sessions_per_era: u64 reclaim_rebate: 0, session_reward: reward, early_era_slash: if monied { 20 } else { 0 }, + offline_slash_grace: 0, }.build_storage().unwrap()); t.extend(timestamp::GenesisConfig::{ period: 5 diff --git a/substrate/runtime/staking/src/tests.rs b/substrate/runtime/staking/src/tests.rs index ef9fa4faa5173..93a81ca8db8e3 100644 --- a/substrate/runtime/staking/src/tests.rs +++ b/substrate/runtime/staking/src/tests.rs @@ -22,6 +22,137 @@ use super::*; use runtime_io::with_externalities; use mock::{Session, Staking, System, Timestamp, Test, new_test_ext}; +#[test] +fn note_null_missed_proposal_should_work() { + with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || { + assert_eq!(Staking::offline_slash_grace(), 0); + assert_eq!(Staking::slash_count(&10), 0); + assert_eq!(Staking::free_balance(&10), 1); + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![])); + assert_eq!(Staking::slash_count(&10), 0); + assert_eq!(Staking::free_balance(&10), 1); + assert!(Staking::forcing_new_era().is_none()); + }); +} + +#[test] +fn note_missed_proposal_should_work() { + with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || { + Staking::set_free_balance(&10, 70); + assert_eq!(Staking::offline_slash_grace(), 0); + assert_eq!(Staking::slash_count(&10), 0); + assert_eq!(Staking::free_balance(&10), 70); + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0])); + assert_eq!(Staking::slash_count(&10), 1); + assert_eq!(Staking::free_balance(&10), 50); + assert!(Staking::forcing_new_era().is_none()); + }); +} + +#[test] +fn note_missed_proposal_exponent_should_work() { + with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || { + Staking::set_free_balance(&10, 150); + assert_eq!(Staking::offline_slash_grace(), 0); + assert_eq!(Staking::slash_count(&10), 0); + assert_eq!(Staking::free_balance(&10), 150); + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0])); + assert_eq!(Staking::slash_count(&10), 1); + assert_eq!(Staking::free_balance(&10), 130); + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0])); + assert_eq!(Staking::slash_count(&10), 2); + assert_eq!(Staking::free_balance(&10), 90); + assert!(Staking::forcing_new_era().is_none()); + }); +} + +#[test] +fn note_missed_proposal_grace_should_work() { + with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || { + Staking::set_free_balance(&10, 70); + Staking::set_free_balance(&20, 70); + assert_ok!(Staking::set_offline_slash_grace(1)); + assert_eq!(Staking::offline_slash_grace(), 1); + + assert_eq!(Staking::slash_count(&10), 0); + assert_eq!(Staking::free_balance(&10), 70); + + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0])); + assert_eq!(Staking::slash_count(&10), 1); + assert_eq!(Staking::free_balance(&10), 70); + assert_eq!(Staking::slash_count(&20), 0); + assert_eq!(Staking::free_balance(&20), 70); + + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0, 1])); + assert_eq!(Staking::slash_count(&10), 2); + assert_eq!(Staking::free_balance(&10), 50); + assert_eq!(Staking::slash_count(&20), 1); + assert_eq!(Staking::free_balance(&20), 70); + assert!(Staking::forcing_new_era().is_none()); + }); +} + +#[test] +fn note_missed_proposal_force_unstake_session_change_should_work() { + with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || { + Staking::set_free_balance(&10, 70); + Staking::set_free_balance(&20, 70); + assert_ok!(Staking::stake(&10)); + assert_ok!(Staking::stake(&20)); + assert_ok!(Staking::stake(&1)); + + assert_eq!(Staking::slash_count(&10), 0); + assert_eq!(Staking::free_balance(&10), 70); + assert_eq!(Staking::intentions(), vec![10, 20, 1]); + assert_eq!(Session::validators(), vec![10, 20]); + + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0])); + assert_eq!(Staking::free_balance(&10), 50); + assert_eq!(Staking::slash_count(&10), 1); + assert_eq!(Staking::intentions(), vec![10, 20, 1]); + + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0])); + assert_eq!(Staking::intentions(), vec![1, 20]); + assert_eq!(Staking::free_balance(&10), 10); + assert!(Staking::forcing_new_era().is_some()); + }); +} + +#[test] +fn note_missed_proposal_auto_unstake_session_change_should_work() { + with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || { + Staking::set_free_balance(&10, 7000); + Staking::set_free_balance(&20, 7000); + assert_ok!(Staking::stake(&10)); + assert_ok!(Staking::stake(&20)); + assert_ok!(Staking::register_slash_preference(&10, 0, SlashPreference { unstake_threshold: 1 })); + + assert_eq!(Staking::intentions(), vec![10, 20]); + + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0, 1])); + assert_eq!(Staking::free_balance(&10), 6980); + assert_eq!(Staking::free_balance(&20), 6980); + assert_eq!(Staking::intentions(), vec![10, 20]); + assert!(Staking::forcing_new_era().is_none()); + + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![0, 1])); + assert_eq!(Staking::free_balance(&10), 6940); + assert_eq!(Staking::free_balance(&20), 6940); + assert_eq!(Staking::intentions(), vec![20]); + assert!(Staking::forcing_new_era().is_some()); + + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![1])); + assert_eq!(Staking::free_balance(&10), 6940); + assert_eq!(Staking::free_balance(&20), 6860); + assert_eq!(Staking::intentions(), vec![20]); + + assert_ok!(Staking::note_missed_proposal(&Default::default(), vec![1])); + assert_eq!(Staking::free_balance(&10), 6940); + assert_eq!(Staking::free_balance(&20), 6700); + assert_eq!(Staking::intentions(), vec![0u64; 0]); + }); +} + #[test] fn reward_should_work() { with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || {