Skip to content

Commit

Permalink
Wrap apply_inactive_safe and apply_inactive_unsafe for ensuring activ…
Browse files Browse the repository at this point in the history
…e intentions and validators count (paritytech#776)

* Wrap apply_inactive_safe and apply_inactive_unsafe for ensuring active
intentions and validators count

* Refactor slash.rs as well for better semantics

* Use try_force_inactive instead of *_unsafe since it's seemingly more
Rust idiomatic

* Rename to force_inactive_unsafe

* Build wasm

* Use Self::try_slash_or_clear for slashing inactive offline validators

* Build wasm
  • Loading branch information
liuchengxu authored Jul 11, 2019
1 parent 76e1069 commit 6d81b98
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 38 deletions.
Binary file modified cli/src/chainx_runtime.compact.wasm
Binary file not shown.
Binary file not shown.
31 changes: 23 additions & 8 deletions xrml/xmining/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,13 +205,8 @@ decl_module! {
}

if let Some(desire_to_run) = desire_to_run.as_ref() {
if !desire_to_run {
let active = Self::intention_set().into_iter()
.filter(|n| Self::is_active(n))
.collect::<Vec<_>>();
if active.len() <= Self::minimum_validator_count() as usize {
return Err("Cannot pull out when there are too few active intentions.");
}
if !desire_to_run && !Self::is_able_to_apply_inactive() {
return Err("Cannot pull out when there are too few active intentions.");
}
}

Expand Down Expand Up @@ -409,14 +404,34 @@ impl<T: Trait> Module<T> {
.collect()
}

pub fn is_able_to_apply_inactive() -> bool {
let active = Self::intention_set()
.into_iter()
.filter(|n| Self::is_active(n))
.collect::<Vec<_>>();
active.len() > Self::minimum_validator_count() as usize
}

// Private mutables
fn force_to_be_inactive(who: &T::AccountId) {
fn apply_inactive(who: &T::AccountId) {
<xaccounts::IntentionPropertiesOf<T>>::mutate(who, |props| {
props.is_active = false;
props.last_inactive_since = <system::Module<T>>::block_number();
});
}

fn force_inactive_unsafe(who: &T::AccountId) {
Self::apply_inactive(who);
}

fn try_force_inactive(who: &T::AccountId) -> Result {
if !Self::is_able_to_apply_inactive() {
return Err("Cannot force inactive when there are too few active intentions.");
}
Self::apply_inactive(who);
Ok(())
}

fn mutate_nomination_record(
nominator: &T::AccountId,
nominee: &T::AccountId,
Expand Down
3 changes: 1 addition & 2 deletions xrml/xmining/staking/src/shifter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ impl<T: Trait> Module<T> {
let satisfy_the_threshold = Self::self_bonded_of(who) >= self_bonded
&& Self::total_nomination_of(who) >= total_bonded;

if !satisfy_the_threshold {
if !satisfy_the_threshold && Self::try_force_inactive(who).is_ok() {
info!("[meet_candidate_threshold] force {:?} to be inactive since it doesn't meet the minimum candidate threshold", who!(who));
Self::force_to_be_inactive(who);
}

satisfy_the_threshold
Expand Down
75 changes: 47 additions & 28 deletions xrml/xmining/staking/src/slash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,41 @@ use rstd::result;
use xsupport::info;

impl<T: Trait> Module<T> {
/// Actually slash the account being punished, all the slashed value will go to the council.
fn apply_slash(slashed_account: &T::AccountId, value: T::Balance) {
let council = xaccounts::Module::<T>::council_account();
let _ = <xassets::Module<T>>::pcx_move_free_balance(&slashed_account, &council, value);
}

/// Slash the total balance of misbehavior's jackpot, return the actually slashed value.
///
/// This is considered be be successful always.
fn slash_whole_jackpot(who: &T::AccountId) -> T::Balance {
let jackpot = Self::jackpot_accountid_for_unsafe(who);
let slashed = <xassets::Module<T>>::pcx_free_balance(&jackpot);
Self::apply_slash(&jackpot, slashed);
slashed
}

/// Try slash given the intention account and value, otherwise empty the whole jackpot.
///
/// If the balance of its jackpot is unable to pay, just slash it all and return the actually slash value.
fn try_slash_or_clear(
who: &T::AccountId,
should_slash: T::Balance,
) -> result::Result<(), T::Balance> {
let jackpot_account = T::DetermineIntentionJackpotAccountId::accountid_for_unsafe(who);
let jackpot_balance = <xassets::Module<T>>::pcx_free_balance(&jackpot_account);

if should_slash <= jackpot_balance {
Self::apply_slash(&jackpot_account, should_slash);
Ok(())
} else {
Self::apply_slash(&jackpot_account, jackpot_balance);
Err(jackpot_balance)
}
}

/// Slash the double signer and return the slashed balance.
///
/// TODO extract the similar slashing logic in shifter.rs.
Expand All @@ -13,20 +48,17 @@ impl<T: Trait> Module<T> {
}

// Slash the whole jackpot of double signer.
let council = xaccounts::Module::<T>::council_account();
let jackpot = Self::jackpot_accountid_for_unsafe(who);

let slashed = <xassets::Module<T>>::pcx_free_balance(&jackpot);
let _ = <xassets::Module<T>>::pcx_move_free_balance(&jackpot, &council, slashed);
let slashed = Self::slash_whole_jackpot(who);
info!(
"[slash_double_signer] {:?} is slashed: {:?}",
who!(who),
slashed
);

// Force the double signer to be inactive.
info!("[slash_double_signer] force {:?} to be inactive", who!(who));
Self::force_to_be_inactive(who);
if Self::try_force_inactive(who).is_ok() {
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| {
Expand All @@ -52,8 +84,6 @@ impl<T: Trait> Module<T> {
my_reward: T::Balance,
validators: &mut Vec<T::AccountId>,
) {
let council = xaccounts::Module::<T>::council_account();

// Slash 10 times per block reward for each missed block.
let missed = u64::from(<MissedOfPerSession<T>>::take(who));
let reward_per_block = Self::reward_of_per_block(my_reward);
Expand All @@ -64,16 +94,12 @@ impl<T: Trait> Module<T> {
T::Balance::sa(Self::minimum_penalty().as_() * missed),
);

let jackpot_addr = T::DetermineIntentionJackpotAccountId::accountid_for_unsafe(who);
let jackpot_balance = <xassets::Module<T>>::pcx_free_balance(&jackpot_addr);

let (slashed, should_be_enforced) = if total_slash <= jackpot_balance {
(total_slash, false)
} else {
(jackpot_balance, true)
};

let _ = <xassets::Module<T>>::pcx_move_free_balance(&jackpot_addr, &council, slashed);
let (slashed, should_be_enforced) =
if let Err(slashed) = Self::try_slash_or_clear(who, total_slash) {
(slashed, true)
} else {
(total_slash, false)
};

debug!(
"[slash_active_offline_validator] {:?} is actually slashed: {:?}, should be slashed: {:?}",
Expand All @@ -89,7 +115,7 @@ impl<T: Trait> Module<T> {
"[slash_active_offline_validator] validator enforced to be inactive: {:?}",
who!(who)
);
Self::force_to_be_inactive(who);
Self::force_inactive_unsafe(who);

// remove from the current validator set
validators.retain(|x| *x != *who);
Expand Down Expand Up @@ -123,14 +149,7 @@ impl<T: Trait> Module<T> {
for who in inactive_slashed.iter() {
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();

let jackpot_addr = T::DetermineIntentionJackpotAccountId::accountid_for_unsafe(who);
let jackpot_balance = <xassets::Module<T>>::pcx_free_balance(&jackpot_addr);

let slash = cmp::min(should_slash, jackpot_balance);

let _ = <xassets::Module<T>>::pcx_move_free_balance(&jackpot_addr, &council, slash);
let _ = Self::try_slash_or_clear(who, should_slash);
}
}
}

0 comments on commit 6d81b98

Please sign in to comment.