Add EvilValidatorsPerSession and fix other nits (paritytech#710)

* Add EvilValidatorsPerSession and fix other nits

* Split out slash.rs in staking

xrml/xfisher/src/lib.rs

@@ -12,9 +12,9 @@ use support::{decl_event, decl_module, decl_storage, dispatch::Result, StorageMa
 use system::ensure_signed;
 
 // ChainX
-use xsupport::{debug, ensure_with_errorlog, error, info, warn};
 #[cfg(feature = "std")]
-use xsupport::{u8array_to_hex, who};
+use xsupport::u8array_to_hex;
+use xsupport::{debug, ensure_with_errorlog, error, info, warn};
 
 pub trait Trait: xstaking::Trait {
     /// The overarching event type.
@@ -79,7 +79,7 @@ decl_module! {
 
             let (fst_height, snd_height) = T::CheckHeader::check_header(&double_signer, &fst_header, &snd_header)?;
 
-            Self::slash(&double_signer, fst_height, snd_height, fst_header.1);
+            let _ = Self::slash(&double_signer, fst_height, snd_height, fst_header.1);
 
             <Reported<T>>::insert(&fst_header.2, ());
             <Reported<T>>::insert(&snd_header.2, ());
@@ -127,49 +127,25 @@ decl_event!(
 );
 
 impl<T: Trait> Module<T> {
-    /// Actually slash the double signer.
-    fn apply_slash(who: &T::AccountId) -> T::Balance {
-        // Slash the whole jackpot of double signer.
-        let council = xaccounts::Module::<T>::council_account();
-        let jackpot = xstaking::Module::<T>::jackpot_accountid_for(who);
-
-        let slashed = <xassets::Module<T>>::pcx_free_balance(&jackpot);
-        let _ = <xassets::Module<T>>::pcx_move_free_balance(&jackpot, &council, slashed);
-        info!(
-            "[slash_double_signer] {:?} is slashed: {:?}",
-            who!(who),
-            slashed
-        );
-
-        // Force the double signer to be inactive.
-        <xaccounts::IntentionPropertiesOf<T>>::mutate(who, |props| {
-            props.is_active = false;
-            props.last_inactive_since = <system::Module<T>>::block_number();
-            info!("[slash_double_signer] force {:?} to be inactive", who!(who));
-        });
-
-        slashed
-    }
-
     fn slash(
         double_signed_key: &T::SessionKey,
         fst_height: T::BlockNumber,
         snd_height: T::BlockNumber,
         slot: u64,
-    ) {
+    ) -> Result {
         if let Some(who) = xsession::Module::<T>::account_id_for(double_signed_key) {
-            if !xstaking::Module::<T>::is_intention(&who) {
+            if let Ok(slashed) = xstaking::Module::<T>::slash_double_signer(&who) {
+                Self::deposit_event(RawEvent::SlashDoubleSigner(
+                    fst_height, snd_height, slot, who, slashed,
+                ));
+                Ok(())
+            } else {
                 warn!("[slash] Try to slash only to find that it is not an intention|session_key:{:?}|accountid:{:?}", double_signed_key, who);
-                return;
+                Err("Fail to slash the double signer")
             }
-
-            let slashed = Self::apply_slash(&who);
-
-            Self::deposit_event(RawEvent::SlashDoubleSigner(
-                fst_height, snd_height, slot, who, slashed,
-            ));
         } else {
             error!("[slash] Cannot find the account id given the double signed session key|session_key:{:?}", double_signed_key);
+            Err("Cannot find the account id given the double signed key")
         }
     }
 }

xrml/xgrandpa/src/lib.rs

@@ -242,6 +242,7 @@ impl<T: Trait> Module<T> {
         }
     }
 
+    #[allow(dead_code)]
     /// Deposit one of this module's logs.
     fn deposit_log(log: Log<T>) {
         <system::Module<T>>::deposit_log(<T as Trait>::Log::from(log).into());

xrml/xmining/staking/src/lib.rs

@@ -5,6 +5,7 @@
 
 mod mock;
 mod shifter;
+pub mod slash;
 mod tests;
 pub mod types;
 pub mod vote_weight;
@@ -335,6 +336,9 @@ decl_storage! {
 
         pub NominationRecords get(nomination_records): map (T::AccountId, T::AccountId) => Option<NominationRecord<T::Balance, T::BlockNumber>>;
 
+        /// Reported validators that did evil, reset per session.
+        pub EvilValidatorsPerSession get(evil_validators): Vec<T::AccountId>;
+
         /// Minimum penalty for each slash.
         pub MinimumPenalty get(minimum_penalty) config(): T::Balance;
         /// The active validators that have ever been offline per session.

xrml/xmining/staking/src/shifter.rs

@@ -84,10 +84,12 @@ impl<T: Trait> Module<T> {
         let council = xaccounts::Module::<T>::council_account();
 
         // Slash 10 times per block reward for each missed block.
-        let missed = <MissedOfPerSession<T>>::take(who) as u64;
+        let missed = u64::from(<MissedOfPerSession<T>>::take(who));
         let reward_per_block = Self::reward_of_per_block(my_reward);
         let total_slash = cmp::max(
-            T::Balance::sa(reward_per_block.as_() * missed * Self::missed_blocks_severity() as u64),
+            T::Balance::sa(
+                reward_per_block.as_() * missed * u64::from(Self::missed_blocks_severity()),
+            ),
             T::Balance::sa(Self::minimum_penalty().as_() * missed),
         );
 
@@ -151,7 +153,7 @@ impl<T: Trait> Module<T> {
         ));
 
         for who in inactive_slashed.iter() {
-            let missed = T::Balance::sa(<MissedOfPerSession<T>>::take(who) as u64);
+            let missed = T::Balance::sa(u64::from(<MissedOfPerSession<T>>::take(who)));
             let should_slash = missed * Self::minimum_penalty();
             let council = xaccounts::Module::<T>::council_account();
 
@@ -191,6 +193,10 @@ impl<T: Trait> Module<T> {
 
         let current_validator_count = validators.len();
 
+        // Try removing the evil validators first.
+        let evil_validators = <EvilValidatorsPerSession<T>>::take();
+        validators.retain(|x| !evil_validators.contains(x));
+
         // apply good session reward
         let mut session_reward = Self::this_session_reward();
 
@@ -227,10 +233,12 @@ impl<T: Trait> Module<T> {
             // May become zero after meeting the last one.
             if !total_active_stake.is_zero() {
                 // stake * session_reward could overflow.
-                let reward = match (stake.as_() as u128).checked_mul(session_reward.as_() as u128) {
+                let reward = match (u128::from(stake.as_()))
+                    .checked_mul(u128::from(session_reward.as_()))
+                {
                     Some(x) => {
-                        let r = x / total_active_stake.as_() as u128;
-                        if r < u64::max_value() as u128 {
+                        let r = x / u128::from(total_active_stake.as_());
+                        if r < u128::from(u64::max_value()) {
                             T::Balance::sa(r as u64)
                         } else {
                             panic!("reward of per intention definitely less than u64::max_value()")
@@ -272,10 +280,8 @@ impl<T: Trait> Module<T> {
 
         if is_new_era {
             Self::new_era();
-        } else {
-            if validators.len() < current_validator_count {
-                Self::set_validators_on_non_era(validators);
-            }
+        } else if validators.len() < current_validator_count {
+            Self::set_validators_on_non_era(validators);
         }
     }
 
@@ -325,7 +331,7 @@ impl<T: Trait> Module<T> {
         }
 
         for (total_nomination, intention) in candidates.iter() {
-            <StakeWeight<T>>::insert(intention, total_nomination.clone());
+            <StakeWeight<T>>::insert(intention, *total_nomination);
         }
 
         let desired_validator_count = <ValidatorCount<T>>::get() as usize;

xrml/xmining/staking/src/slash.rs

@@ -0,0 +1,42 @@
+use super::*;
+use rstd::result;
+use xsupport::info;
+
+impl<T: Trait> Module<T> {
+    /// Slash the double signer and return the slashed balance.
+    ///
+    /// TODO extract the similar slashing logic in shifter.rs.
+    pub fn slash_double_signer(who: &T::AccountId) -> result::Result<T::Balance, &'static str> {
+        if !Self::is_intention(who) {
+            return Err("Cannot slash if the reported double signer is not an intention");
+        }
+
+        // Slash the whole jackpot of double signer.
+        let council = xaccounts::Module::<T>::council_account();
+        let jackpot = Self::jackpot_accountid_for(who);
+
+        let slashed = <xassets::Module<T>>::pcx_free_balance(&jackpot);
+        let _ = <xassets::Module<T>>::pcx_move_free_balance(&jackpot, &council, slashed);
+        info!(
+            "[slash_double_signer] {:?} is slashed: {:?}",
+            who!(who),
+            slashed
+        );
+
+        // Force the double signer to be inactive.
+        <xaccounts::IntentionPropertiesOf<T>>::mutate(who, |props| {
+            props.is_active = false;
+            props.last_inactive_since = <system::Module<T>>::block_number();
+            info!("[slash_double_signer] force {:?} to be inactive", who!(who));
+        });
+
+        // Note the double signer so that he could be removed from the current validator set on new session.
+        <EvilValidatorsPerSession<T>>::mutate(|evil_validators| {
+            if !evil_validators.contains(&who) {
+                evil_validators.push(who.clone())
+            }
+        });
+
+        Ok(slashed)
+    }
+}