Fix/various fixes and improvement (paritytech#306)

* Only accept these intentions with a related TrusteeEntity

- Fixes paritytech#292
- also change MAX trustee intentions from 16 to 15

* Skip set_validators when the validator set stays the same

Fixes paritytech#295

* Enforce these punished to be inactive on new session instead of new era

Fixes paritytech#294

* Make items in the punish list unique

- Fixes paritytech#307

- Also refactor a bit in genesis_config.rs

- Return if the count of candidates less than minimal validator count on
enforce_inactive.

* Store trustee_count and minimum_trustee_count in Runtime

- also tweak some parameters.

* Add new event: EnforceValidatorsInactive

* Rebuild wasm

* Build wasm

cli/src/genesis_config.rs

@@ -159,6 +159,12 @@ pub fn testnet_genesis(genesis_spec: GenesisSpec) -> GenesisConfig {
         .map(|(auth, balance, _, _, _, _)| (auth, balance))
         .collect::<Vec<_>>();
 
+    let blocks_per_session = 150; // 150 blocks per session
+    let sessions_per_era = 12; // update validators set per 12 sessions
+    let sessions_per_epoch = sessions_per_era * 10; // update trustees set per 12*10 sessions
+    let bonding_duration = blocks_per_session * sessions_per_era; // freeze 150*12 blocks for non-intention
+    let intention_bonding_duration = bonding_duration * 10; // freeze 150*12*10 blocks for intention
+
     GenesisConfig {
         consensus: Some(ConsensusConfig {
             code: include_bytes!(
@@ -176,7 +182,7 @@ pub fn testnet_genesis(genesis_spec: GenesisSpec) -> GenesisConfig {
         }),
         session: Some(SessionConfig {
             validators: endowed.iter().cloned().map(|(account, balance)| (account.into(), balance)).collect(),
-            session_length: 150, // 150 blocks per session
+            session_length: blocks_per_session,
         }),
         sudo: Some(SudoConfig {
             key: auth1.into(),
@@ -211,12 +217,14 @@ pub fn testnet_genesis(genesis_spec: GenesisSpec) -> GenesisConfig {
         }),
         xstaking: Some(XStakingConfig {
             initial_reward: apply_prec(50.0),
-            validator_count: 100,
+            validator_count: 30,
             minimum_validator_count: 4,
-            sessions_per_era: 12,  // update validators set per 12 sessions
-            sessions_per_epoch: 12 * 10, // update trustees set per 120 sessions
-            bonding_duration: 150 * 12, // 150 blocks per bonding
-            intention_bonding_duration: 150 * 12 * 10,
+            trustee_count: 4,
+            minimum_trustee_count: 4,
+            sessions_per_era: sessions_per_era,
+            sessions_per_epoch: sessions_per_epoch,
+            bonding_duration: bonding_duration,
+            intention_bonding_duration: intention_bonding_duration,
             current_era: 0,
             penalty: 50 * 100_000_000 / 150, // 1 per block reward
             funding: Default::default(),

xrml/xmining/staking/src/lib.rs

@@ -63,8 +63,6 @@ pub use shifter::{OnReward, OnRewardCalculation, RewardHolder};
 pub use vote_weight::VoteWeight;
 
 const DEFAULT_MINIMUM_VALIDATOR_COUNT: u32 = 4;
-const MIMIMUM_TRSUTEE_INTENSION_COUNT: u32 = 4;
-const MAXIMUM_TRSUTEE_INTENSION_COUNT: u32 = 16;
 
 /// Intention mutable properties
 #[derive(PartialEq, Eq, Clone, Encode, Decode, Default)]
@@ -319,6 +317,7 @@ decl_event!(
         /// One validator (and their nominators) has been slashed by the given amount.
         OfflineSlash(AccountId, Balance),
         OfflineValidator(AccountId),
+        EnforceValidatorsInactive(Vec<AccountId>),
         Rotation(Vec<(AccountId, u64)>),
         NewTrustees(Vec<AccountId>),
         Unnominate(BlockNumber),
@@ -331,7 +330,11 @@ decl_event!(
 
 decl_storage! {
     trait Store for Module<T: Trait> as XStaking {
-        InitialReward get(initial_reward) config(): T::Balance;
+        pub InitialReward get(initial_reward) config(): T::Balance;
+
+        pub TrusteeCount get(trustee_count) config(): u32;
+        pub MinimumTrusteeCount get(minimum_trustee_count) config(): u32;
+
         /// The ideal number of staking participants.
         pub ValidatorCount get(validator_count) config(): u32;
         /// Minimum number of staking participants before emergency conditions are imposed.

xrml/xmining/staking/src/mock.rs

@@ -188,10 +188,13 @@ pub fn new_test_ext() -> runtime_io::TestExternalities<Blake2Hasher> {
                 .collect(),
             current_era: 0,
             validator_count: 2,
+            minimum_validator_count: 0,
+            trustee_count: 8,
+            minimum_trustee_count: 4,
             bonding_duration: 1,
             intention_bonding_duration: 10,
-            minimum_validator_count: 0,
             sessions_per_era: 1,
+            sessions_per_epoch: 10,
             funding: 10,
             penalty: 10,
             validator_stake_threshold: 1,
@@ -201,7 +204,7 @@ pub fn new_test_ext() -> runtime_io::TestExternalities<Blake2Hasher> {
                     (who.into(), hot_entity, cold_entity)
                 })
                 .collect(),
-            team: 100,
+            team_address: 100,
         }
         .build_storage()
         .unwrap()

xrml/xmining/staking/src/shifter.rs

@@ -82,20 +82,44 @@ impl<T: Trait> Module<T> {
         let _ = <xassets::Module<T>>::pcx_issue(&jackpot_addr, to_jackpot);
     }
 
-    /// Punish  a given (potential) validator by a specific amount.
-    fn punish(who: &T::AccountId, punish: T::Balance) -> bool {
+    /// Slash a given (potential) validator by a specific amount.
+    fn slash_validator(who: &T::AccountId, slash: T::Balance) {
         let jackpot_addr = T::DetermineIntentionJackpotAccountId::accountid_for(who);
         let fund_id = Self::funding();
-        if punish <= <xassets::Module<T>>::pcx_free_balance(&jackpot_addr) {
-            let _ = <xassets::Module<T>>::pcx_move_free_balance(&jackpot_addr, &fund_id, punish);
-            return true;
+
+        if slash <= <xassets::Module<T>>::pcx_free_balance(&jackpot_addr) {
+            let _ = <xassets::Module<T>>::pcx_move_free_balance(&jackpot_addr, &fund_id, slash);
+        } else {
+            // Put the slashed validator into the punished list when its jackpot is not enough to be slashed.
+            // These on the punish list will be enforced inactive on new session when possible.
+            if !<PunishList<T>>::get().into_iter().any(|x| x == *who) {
+                <PunishList<T>>::mutate(|i| i.push(who.clone()));
+            }
+        }
+    }
+
+    /// Enforce these punished to be inactive, so that they won't become validators and be rewarded.
+    fn enforce_inactive() {
+        if Self::gather_candidates().len() <= Self::minimum_validator_count() as usize {
+            return;
         }
-        return false;
+
+        let punished = <PunishList<T>>::take();
+        for v in punished.iter() {
+            // Force those punished to be inactive
+            <xaccounts::IntentionPropertiesOf<T>>::mutate(v, |props| {
+                props.is_active = false;
+            });
+        }
+
+        Self::deposit_event(RawEvent::EnforceValidatorsInactive(punished));
     }
 
     /// 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) {
+        Self::enforce_inactive();
+
         if should_reward {
             // apply good session reward
             let mut session_reward = Self::this_session_reward();
@@ -178,15 +202,6 @@ impl<T: Trait> Module<T> {
             }
         }
 
-        let punish_list = <PunishList<T>>::take();
-        for punished in punish_list {
-            // Force those punished to be inactive
-            <xaccounts::IntentionPropertiesOf<T>>::mutate(&punished, |props| {
-                props.is_active = false;
-            });
-            Self::deposit_event(RawEvent::OfflineValidator(punished));
-        }
-
         // 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
@@ -206,32 +221,46 @@ impl<T: Trait> Module<T> {
 
         let desired_validator_count = <ValidatorCount<T>>::get() as usize;
 
-        let vals = candidates
+        let old_validators = <session::Module<T>>::validators();
+
+        let new_validators = candidates
             .into_iter()
             .take(desired_validator_count)
             .map(|(stake_weight, account_id)| (account_id, stake_weight.as_()))
             .collect::<Vec<(_, _)>>();
 
-        <session::Module<T>>::set_validators(&vals);
+        // Skip set validators when the validator set stays the same.
+        if old_validators.len() == new_validators.len()
+            && old_validators
+                .iter()
+                .map(|(val, _)| val)
+                .zip(new_validators.iter().map(|(val, _)| val))
+                .all(|(a, b)| a == b)
+        {
+            return;
+        }
 
-        Self::deposit_event(RawEvent::Rotation(vals.clone()));
+        <session::Module<T>>::set_validators(&new_validators);
+        Self::deposit_event(RawEvent::Rotation(new_validators.clone()));
     }
 
     pub fn on_offline_validator(v: &T::AccountId) {
-        let penalty = Self::penalty();
-        if Self::punish(v, penalty) == false {
-            <PunishList<T>>::mutate(|i| i.push(v.clone()));
-        }
-        Self::deposit_event(RawEvent::OfflineSlash(v.clone(), penalty));
+        let slash = Self::penalty();
+        Self::slash_validator(v, slash);
+        Self::deposit_event(RawEvent::OfflineSlash(v.clone(), slash));
     }
 
     fn new_trustees() {
         let intentions = Self::gather_candidates();
-        if intentions.len() as u32 >= MIMIMUM_TRSUTEE_INTENSION_COUNT {
+        if intentions.len() as u32 >= Self::minimum_trustee_count() {
             let trustees = intentions
                 .into_iter()
-                .take(MAXIMUM_TRSUTEE_INTENSION_COUNT as usize)
+                .take(Self::trustee_count() as usize)
                 .map(|(_, v)| v)
+                .filter(|v| {
+                    <xaccounts::TrusteeIntentionPropertiesOf<T>>::get(&(v.clone(), Chain::Bitcoin))
+                        .is_some()
+                })
                 .collect::<Vec<_>>();
 
             <xaccounts::TrusteeIntentions<T>>::put(trustees.clone());

xrml/xmining/staking/src/tests.rs

@@ -134,6 +134,61 @@ fn unnominate_should_work() {
     });
 }
 
+#[test]
+fn new_trustees_should_work() {
+    with_externalities(&mut new_test_ext(), || {
+        System::set_block_number(1);
+        Session::check_rotate_session(System::block_number());
+
+        assert_ok!(Staking::register(Origin::signed(1), b"name".to_vec()));
+        assert_ok!(Staking::nominate(Origin::signed(1), 1.into(), 5, vec![]));
+        assert_ok!(Staking::refresh(
+            Origin::signed(1),
+            Some(b"new.name".to_vec()),
+            Some(true),
+            Some(UintAuthorityId(123).into()),
+            None
+        ));
+        assert_ok!(Staking::setup_trustee(
+            Origin::signed(1),
+            Chain::Bitcoin,
+            b"about".to_vec(),
+            TrusteeEntity::Bitcoin(vec![0; 33]),
+            TrusteeEntity::Bitcoin(vec![0; 33]),
+        ));
+
+        System::set_block_number(10);
+        Session::check_rotate_session(System::block_number());
+
+        System::set_block_number(11);
+        Session::check_rotate_session(System::block_number());
+
+        System::set_block_number(12);
+        Session::check_rotate_session(System::block_number());
+
+        System::set_block_number(13);
+        Session::check_rotate_session(System::block_number());
+
+        System::set_block_number(14);
+        Session::check_rotate_session(System::block_number());
+
+        System::set_block_number(15);
+        Session::check_rotate_session(System::block_number());
+
+        System::set_block_number(16);
+        Session::check_rotate_session(System::block_number());
+
+        System::set_block_number(17);
+        Session::check_rotate_session(System::block_number());
+        assert_eq!(XAccounts::trustee_intentions(), [10, 20, 30, 40]);
+
+        System::set_block_number(18);
+        Session::check_rotate_session(System::block_number());
+        assert_eq!(Session::current_index(), 10);
+        assert_eq!(XAccounts::trustee_intentions(), [30, 40, 20, 10, 1]);
+    });
+}
+
 /*
 #[test]
 fn unfreeze_should_work() {