From 9609fda60a7c818912be8e03353292d5bc0d487f Mon Sep 17 00:00:00 2001 From: Gav Date: Tue, 4 Sep 2018 17:54:26 +0200 Subject: [PATCH 1/4] Somerhing wrong. --- substrate/runtime/staking/src/lib.rs | 33 ++++++++++++---------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/substrate/runtime/staking/src/lib.rs b/substrate/runtime/staking/src/lib.rs index 18fea81cf5a99..b2d138456a87d 100644 --- a/substrate/runtime/staking/src/lib.rs +++ b/substrate/runtime/staking/src/lib.rs @@ -50,7 +50,7 @@ use runtime_support::{Parameter, StorageValue, StorageMap}; use runtime_support::dispatch::Result; use session::OnSessionChange; use primitives::traits::{Zero, One, Bounded, RefInto, OnFinalise, - As, AuxLookup}; + As, AuxLookup, Member}; use balances::{address::Address, OnMinted}; mod mock; @@ -75,18 +75,19 @@ pub enum LockStatus { LockedUntil(BlockNumber), Bonded, } -/* + /// Preference of what happens on a slash event. -#[cfg_attr(feature = "std", derive(Debug, Serialize, Deserialize))] -#[derive(Encode, Decode, Eq, PartialEq, Clone, Copy)] -pub struct ValidatorPrefs { +#[derive(PartialEq, Eq, Clone, Encode, Decode)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +pub struct ValidatorPrefs where Balance: Member { /// Validator should ensure this many more slashes than is necessary before being unstaked. pub unstake_threshold: u32, // Reward that validator takes up-front; only the rest is split between themself and nominators. + #[cfg_attr(feature = "std", serde(deserialize_with="Balance::deserialize"))] pub validator_payment: Balance, } -impl Default for ValidatorPrefs { +impl Default for ValidatorPrefs { fn default() -> Self { ValidatorPreferences { unstake_threshold: 3, @@ -94,7 +95,7 @@ impl Default for ValidatorPrefs { } } } -*/ + pub trait Trait: balances::Trait + session::Trait { /// Some tokens minted. type OnRewardMinted: OnMinted<::Balance>; @@ -112,7 +113,7 @@ decl_module! { fn unstake(aux, intentions_index: u32) -> Result = 1; fn nominate(aux, target: Address) -> Result = 2; fn unnominate(aux, target_index: u32) -> Result = 3; - fn register_preferences(aux, intentions_index: u32, unstake_threshold: u32, validator_payment: T::Balance) -> Result = 4; + fn register_preferences(aux, intentions_index: u32, prefs: ValidatorPrefs) -> Result = 4; } #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] @@ -162,7 +163,7 @@ decl_storage! { // The current era index. pub CurrentEra get(current_era): required T::BlockNumber; // Preferences that a validator has. - pub ValidatorPreferences: map [ T::AccountId => (u32, T::Balance) ]; + pub ValidatorPreferences get(validator_preferences): map [ T::AccountId => ValidatorPrefs ]; // All the accounts with a desire to stake. pub Intentions get(intentions): default Vec; // All nominator -> nominee relationships. @@ -193,11 +194,6 @@ impl Module { // PUBLIC IMMUTABLES - /// ValidatorPreferences getter, introduces a default. - pub fn validator_preferences(who: &T::AccountId) -> (u32, T::Balance) { - >::get(who).unwrap_or_else(|| (3, Zero::zero())) - } - /// MinimumValidatorCount getter, introduces a default. pub fn minimum_validator_count() -> usize { >::get().map(|v| v as usize).unwrap_or(DEFAULT_MINIMUM_VALIDATOR_COUNT) @@ -313,8 +309,7 @@ impl Module { fn register_preferences( aux: &T::PublicAux, intentions_index: u32, - unstake_threshold: u32, - validator_payment: T::Balance + prefs: ValidatorPrefs ) -> Result { let aux = aux.ref_into(); @@ -322,7 +317,7 @@ impl Module { return Err("Invalid index") } - >::insert(aux, (unstake_threshold, validator_payment)); + >::insert(aux, prefs); Ok(()) } @@ -391,7 +386,7 @@ impl Module { /// 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 off_the_table = reward.min(Self::validator_preferences(who).1); + let off_the_table = reward.min(Self::validator_preferences(who).off_the_table); let reward = reward - off_the_table; let validator_cut = if reward.is_zero() { Zero::zero() @@ -551,7 +546,7 @@ impl consensus::OnOfflineValidator for Module { let slash = Self::early_era_slash() << instances; let next_slash = slash << 1u32; let _ = Self::slash_validator(&v, slash); - if instances >= Self::validator_preferences(&v).0 + if instances >= Self::validator_preferences(&v).unstake_threshold || Self::slashable_balance(&v) < next_slash { if let Some(pos) = Self::intentions().into_iter().position(|x| &x == &v) { From a763e9055c395fc5015b94b7d682ab35c0e47f95 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Tue, 4 Sep 2018 20:06:58 +0300 Subject: [PATCH 2/4] My attempt to fix --- substrate/runtime/staking/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/substrate/runtime/staking/src/lib.rs b/substrate/runtime/staking/src/lib.rs index b2d138456a87d..bf42c91f379cb 100644 --- a/substrate/runtime/staking/src/lib.rs +++ b/substrate/runtime/staking/src/lib.rs @@ -50,7 +50,7 @@ use runtime_support::{Parameter, StorageValue, StorageMap}; use runtime_support::dispatch::Result; use session::OnSessionChange; use primitives::traits::{Zero, One, Bounded, RefInto, OnFinalise, - As, AuxLookup, Member}; + As, AuxLookup}; use balances::{address::Address, OnMinted}; mod mock; @@ -79,19 +79,18 @@ pub enum LockStatus { /// Preference of what happens on a slash event. #[derive(PartialEq, Eq, Clone, Encode, Decode)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] -pub struct ValidatorPrefs where Balance: Member { +pub struct ValidatorPrefs { /// Validator should ensure this many more slashes than is necessary before being unstaked. pub unstake_threshold: u32, // Reward that validator takes up-front; only the rest is split between themself and nominators. - #[cfg_attr(feature = "std", serde(deserialize_with="Balance::deserialize"))] pub validator_payment: Balance, } -impl Default for ValidatorPrefs { +impl Default for ValidatorPrefs { fn default() -> Self { - ValidatorPreferences { + ValidatorPrefs { unstake_threshold: 3, - off_the_table: Default::default(), + validator_payment: Default::default(), } } } @@ -108,6 +107,7 @@ decl_module! { pub struct Module; #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] + #[serde(bound(deserialize = "T::Balance: ::serde::de::DeserializeOwned"))] pub enum Call where aux: T::PublicAux { fn stake(aux) -> Result = 0; fn unstake(aux, intentions_index: u32) -> Result = 1; @@ -163,7 +163,7 @@ decl_storage! { // The current era index. pub CurrentEra get(current_era): required T::BlockNumber; // Preferences that a validator has. - pub ValidatorPreferences get(validator_preferences): map [ T::AccountId => ValidatorPrefs ]; + pub ValidatorPreferences get(validator_preferences): default map [ T::AccountId => ValidatorPrefs ]; // All the accounts with a desire to stake. pub Intentions get(intentions): default Vec; // All nominator -> nominee relationships. @@ -386,7 +386,7 @@ impl Module { /// 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 off_the_table = reward.min(Self::validator_preferences(who).off_the_table); + let off_the_table = reward.min(Self::validator_preferences(who).validator_payment); let reward = reward - off_the_table; let validator_cut = if reward.is_zero() { Zero::zero() From 29349f78e26f45842cb4562de69c4572f5235d88 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Tue, 4 Sep 2018 20:12:18 +0300 Subject: [PATCH 3/4] cfg_attr for serde --- substrate/runtime/staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/runtime/staking/src/lib.rs b/substrate/runtime/staking/src/lib.rs index bf42c91f379cb..42d35c9c3ec85 100644 --- a/substrate/runtime/staking/src/lib.rs +++ b/substrate/runtime/staking/src/lib.rs @@ -107,7 +107,7 @@ decl_module! { pub struct Module; #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] - #[serde(bound(deserialize = "T::Balance: ::serde::de::DeserializeOwned"))] + #[cfg_attr(feature = "std", serde(bound(deserialize = "T::Balance: ::serde::de::DeserializeOwned")))] pub enum Call where aux: T::PublicAux { fn stake(aux) -> Result = 0; fn unstake(aux, intentions_index: u32) -> Result = 1; From b5ff6f6320b2bd5cde510fcb8f5c5a6461572b35 Mon Sep 17 00:00:00 2001 From: Gav Date: Tue, 4 Sep 2018 21:43:20 +0200 Subject: [PATCH 4/4] Fix tests --- substrate/runtime/staking/src/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/runtime/staking/src/tests.rs b/substrate/runtime/staking/src/tests.rs index 35f203b078535..4bf9ce9f41051 100644 --- a/substrate/runtime/staking/src/tests.rs +++ b/substrate/runtime/staking/src/tests.rs @@ -130,7 +130,7 @@ fn note_offline_auto_unstake_session_change_should_work() { with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || { Balances::set_free_balance(&10, 7000); Balances::set_free_balance(&20, 7000); - assert_ok!(Staking::register_preferences(&10, 0, 1, 0)); + assert_ok!(Staking::register_preferences(&10, 0, ValidatorPrefs { unstake_threshold: 1, validator_payment: 0 })); assert_eq!(Staking::intentions(), vec![10, 20]); @@ -348,15 +348,15 @@ fn rewards_with_off_the_table_should_work() { assert_ok!(Staking::stake(&1)); assert_ok!(Staking::nominate(&2, 1.into())); assert_ok!(Staking::stake(&3)); - Session::check_rotate_session(); + Session::check_rotate_session(System::block_number()); assert_eq!(Session::validators(), vec![1, 3]); // 1 + 2, 3 assert_eq!(Balances::total_balance(&1), 10); assert_eq!(Balances::total_balance(&2), 20); assert_eq!(Balances::total_balance(&3), 30); System::set_block_number(2); - assert_ok!(Staking::register_preferences(&1, Staking::intentions().into_iter().position(|i| i == 1).unwrap() as u32, 3, 4)); - Session::check_rotate_session(); + assert_ok!(Staking::register_preferences(&1, Staking::intentions().into_iter().position(|i| i == 1).unwrap() as u32, ValidatorPrefs { unstake_threshold: 3, validator_payment: 4 })); + Session::check_rotate_session(System::block_number()); assert_eq!(Balances::total_balance(&1), 16); assert_eq!(Balances::total_balance(&2), 24); assert_eq!(Balances::total_balance(&3), 40);