fix overflow in slashing logic (#1263)

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<Self, Output = Self> + DivAssign<Self> +
 	Rem<Self, Output = Self> + RemAssign<Self> +
 	Shl<u32, Output = Self> + Shr<u32, Output = Self> +
+	CheckedShl +
+	CheckedShr +
 	CheckedAdd +
 	CheckedSub +
 	CheckedMul +
@@ -173,6 +177,8 @@ impl<T:
 	Div<Self, Output = Self> + DivAssign<Self> +
 	Rem<Self, Output = Self> + RemAssign<Self> +
 	Shl<u32, Output = Self> + Shr<u32, Output = Self> +
+	CheckedShl +
+	CheckedShr +
 	CheckedAdd +
 	CheckedSub +
 	CheckedMul +

substrate/srml/staking/src/lib.rs

@@ -499,6 +499,8 @@ impl<T: Trait> Module<T> {
 	/// 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);
 			<SlashCount<T>>::insert(v.clone(), slash_count + 1);
@@ -506,8 +508,23 @@ impl<T: Trait> Module<T> {
 
 			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.
+						<SlashCount<T>>::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

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.
+		<ValidatorPreferences<Test>>::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<A> TypeEq for (A, A) {}
+		fn assert_type_eq<A: TypeEq>() {}
+		assert_type_eq::<(u64, <Test as balances::Trait>::Balance)>();
+
+		Staking::on_offline_validator(10, 100);
+	});
+}