diff --git a/substrate/core/sr-primitives/src/traits.rs b/substrate/core/sr-primitives/src/traits.rs index 7c9692cad3..8bdfd7eda6 100644 --- a/substrate/core/sr-primitives/src/traits.rs +++ b/substrate/core/sr-primitives/src/traits.rs @@ -26,7 +26,9 @@ use substrate_primitives::Blake2Hasher; use codec::{Codec, Encode, HasCompact}; pub use integer_sqrt::IntegerSquareRoot; pub use num_traits::{Zero, One, Bounded}; -pub use num_traits::ops::checked::{CheckedAdd, CheckedSub, CheckedMul, CheckedDiv}; +pub use num_traits::ops::checked::{ + CheckedAdd, CheckedSub, CheckedMul, CheckedDiv, CheckedShl, CheckedShr, +}; use rstd::ops::{ Add, Sub, Mul, Div, Rem, AddAssign, SubAssign, MulAssign, DivAssign, RemAssign, Shl, Shr @@ -158,6 +160,8 @@ pub trait SimpleArithmetic: Div + DivAssign + Rem + RemAssign + Shl + Shr + + CheckedShl + + CheckedShr + CheckedAdd + CheckedSub + CheckedMul + @@ -173,6 +177,8 @@ impl + DivAssign + Rem + RemAssign + Shl + Shr + + CheckedShl + + CheckedShr + CheckedAdd + CheckedSub + CheckedMul + diff --git a/substrate/srml/staking/src/lib.rs b/substrate/srml/staking/src/lib.rs index 4982b24de2..208c14fc0f 100644 --- a/substrate/srml/staking/src/lib.rs +++ b/substrate/srml/staking/src/lib.rs @@ -499,6 +499,8 @@ impl Module { /// Call when a validator is determined to be offline. `count` is the /// number of offences the validator has committed. pub fn on_offline_validator(v: T::AccountId, count: usize) { + use primitives::traits::CheckedShl; + for _ in 0..count { let slash_count = Self::slash_count(&v); >::insert(v.clone(), slash_count + 1); @@ -506,8 +508,23 @@ impl Module { let event = if slash_count >= grace { let instances = slash_count - grace; - let slash = Self::current_offline_slash() << instances; - let next_slash = slash << 1u32; + + let base_slash = Self::current_offline_slash(); + let slash = match base_slash.checked_shl(instances) { + Some(slash) => slash, + None => { + // freeze at last maximum valid slash if this starts + // to overflow. + >::insert(v.clone(), slash_count); + base_slash.checked_shl(instances - 1) + .expect("slash count no longer incremented after overflow; \ + prior check only fails with instances >= 1; \ + thus instances - 1 always works and is a valid amount of bits; qed") + } + }; + + let next_slash = slash << 1; + let _ = Self::slash_validator(&v, slash); if instances >= Self::validator_preferences(&v).unstake_threshold || Self::slashable_balance(&v) < next_slash diff --git a/substrate/srml/staking/src/tests.rs b/substrate/srml/staking/src/tests.rs index 1e296ba53f..88ce66c8d7 100644 --- a/substrate/srml/staking/src/tests.rs +++ b/substrate/srml/staking/src/tests.rs @@ -504,3 +504,40 @@ fn deducting_balance_when_bonded_should_not_work() { assert_noop!(Balances::reserve(&1, 69), "cannot transfer illiquid funds"); }); } + +#[test] +fn slash_value_calculation_does_not_overflow() { + with_externalities(&mut new_test_ext(0, 3, 3, 0, true, 10), || { + assert_eq!(Staking::era_length(), 9); + assert_eq!(Staking::sessions_per_era(), 3); + assert_eq!(Staking::last_era_length_change(), 0); + assert_eq!(Staking::current_era(), 0); + assert_eq!(Session::current_index(), 0); + assert_eq!(Balances::total_balance(&10), 1); + assert_eq!(Staking::intentions(), vec![10, 20]); + assert_eq!(Staking::offline_slash_grace(), 0); + + // set validator preferences so the validator doesn't back down after + // slashing. + >::insert(10, ValidatorPrefs { + unstake_threshold: u32::max_value(), + validator_payment: 0, + }); + + System::set_block_number(3); + Session::check_rotate_session(System::block_number()); + assert_eq!(Staking::current_era(), 0); + assert_eq!(Session::current_index(), 1); + assert_eq!(Balances::total_balance(&10), 11); + + // the balance type is u64, so after slashing 64 times, + // the slash value should have overflowed. add a couple extra for + // good measure with the slash grace. + trait TypeEq {} + impl TypeEq for (A, A) {} + fn assert_type_eq() {} + assert_type_eq::<(u64, ::Balance)>(); + + Staking::on_offline_validator(10, 100); + }); +}