safe multi-era slashing for NPoS

core/sr-staking-primitives/src/offence.rs

@@ -117,16 +117,20 @@ pub trait OnOffenceHandler<Reporter, Offender> {
 	/// the authorities should be slashed and is computed
 	/// according to the `OffenceCount` already. This is of the same length as `offenders.`
 	/// Zero is a valid value for a fraction.
+	///
+	/// The `session` parameter is the session index of the offence.
 	fn on_offence(
 		offenders: &[OffenceDetails<Reporter, Offender>],
 		slash_fraction: &[Perbill],
+		session: SessionIndex,
 	);
 }
 
 impl<Reporter, Offender> OnOffenceHandler<Reporter, Offender> for () {
 	fn on_offence(
 		_offenders: &[OffenceDetails<Reporter, Offender>],
 		_slash_fraction: &[Perbill],
+		_session: SessionIndex,
 	) {}
 }
 

node/runtime/src/lib.rs

@@ -81,7 +81,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to equal spec_version. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 189,
+	spec_version: 190,
 	impl_version: 190,
 	apis: RUNTIME_API_VERSIONS,
 };

srml/balances/src/lib.rs

@@ -921,6 +921,7 @@ where
 	) -> (Self::NegativeImbalance, Self::Balance) {
 		let free_balance = Self::free_balance(who);
 		let free_slash = cmp::min(free_balance, value);
+
 		Self::set_free_balance(who, free_balance - free_slash);
 		let remaining_slash = value - free_slash;
 		// NOTE: `slash()` prefers free balance, but assumes that reserve balance can be drawn

srml/offences/src/lib.rs

@@ -24,17 +24,11 @@
 mod mock;
 mod tests;
 
-use rstd::{
-	vec::Vec,
-	collections::btree_set::BTreeSet,
-};
+use rstd::vec::Vec;
 use support::{
 	decl_module, decl_event, decl_storage, Parameter,
 };
-use sr_primitives::{
-	Perbill,
-	traits::{Hash, Saturating},
-};
+use sr_primitives::traits::Hash;
 use sr_staking_primitives::{
 	offence::{Offence, ReportOffence, Kind, OnOffenceHandler, OffenceDetails},
 };
@@ -100,10 +94,11 @@ where
 
 		// Go through all offenders in the offence report and find all offenders that was spotted
 		// in unique reports.
-		let TriageOutcome {
-			new_offenders,
-			concurrent_offenders,
-		} = match Self::triage_offence_report::<O>(reporters, &time_slot, offenders) {
+		let TriageOutcome { concurrent_offenders } = match Self::triage_offence_report::<O>(
+			reporters,
+			&time_slot,
+			offenders,
+		) {
 			Some(triage) => triage,
 			// The report contained only duplicates, so there is no need to slash again.
 			None => return,
@@ -113,44 +108,18 @@ where
 		Self::deposit_event(Event::Offence(O::ID, time_slot.encode()));
 
 		let offenders_count = concurrent_offenders.len() as u32;
-		let previous_offenders_count = offenders_count - new_offenders.len() as u32;
 
 		// The amount new offenders are slashed
 		let new_fraction = O::slash_fraction(offenders_count, validator_set_count);
 
-		// The amount previous offenders are slashed additionally.
-		//
-		// Since they were slashed in the past, we slash by:
-		// x = (new - prev) / (1 - prev)
-		// because:
-		// Y = X * (1 - prev)
-		// Z = Y * (1 - x)
-		// Z = X * (1 - new)
-		let old_fraction = if previous_offenders_count > 0 {
-			let previous_fraction = O::slash_fraction(
-				offenders_count.saturating_sub(previous_offenders_count),
-				validator_set_count,
-			);
-			let numerator = new_fraction.saturating_sub(previous_fraction);
-			let denominator = Perbill::one().saturating_sub(previous_fraction);
-			denominator.saturating_mul(numerator)
-		} else {
-			new_fraction.clone()
-		};
+		let slash_perbill: Vec<_> = (0..concurrent_offenders.len())
+			.map(|_| new_fraction.clone()).collect();
 
-		// calculate how much to slash
-		let slash_perbill = concurrent_offenders
-			.iter()
-			.map(|details| {
-				if previous_offenders_count > 0 && new_offenders.contains(&details.offender) {
-					new_fraction.clone()
-				} else {
-					old_fraction.clone()
-				}
-			})
-			.collect::<Vec<_>>();
-
-		T::OnOffenceHandler::on_offence(&concurrent_offenders, &slash_perbill);
+		T::OnOffenceHandler::on_offence(
+			&concurrent_offenders,
+			&slash_perbill,
+			offence.session_index(),
+		);
 	}
 }
 
@@ -173,13 +142,13 @@ impl<T: Trait> Module<T> {
 		offenders: Vec<T::IdentificationTuple>,
 	) -> Option<TriageOutcome<T>> {
 		let mut storage = ReportIndexStorage::<T, O>::load(time_slot);
-		let mut new_offenders = BTreeSet::new();
 
+		let mut any_new = false;
 		for offender in offenders {
 			let report_id = Self::report_id::<O>(time_slot, &offender);
 
 			if !<Reports<T>>::exists(&report_id) {
-				new_offenders.insert(offender.clone());
+				any_new = true;
 				<Reports<T>>::insert(
 					&report_id,
 					OffenceDetails {
@@ -192,7 +161,7 @@ impl<T: Trait> Module<T> {
 			}
 		}
 
-		if !new_offenders.is_empty() {
+		if any_new {
 			// Load report details for the all reports happened at the same time.
 			let concurrent_offenders = storage.concurrent_reports
 				.iter()
@@ -202,7 +171,6 @@ impl<T: Trait> Module<T> {
 			storage.save();
 
 			Some(TriageOutcome {
-				new_offenders,
 				concurrent_offenders,
 			})
 		} else {
@@ -212,8 +180,6 @@ impl<T: Trait> Module<T> {
 }
 
 struct TriageOutcome<T: Trait> {
-	/// Offenders that was spotted in the unique reports.
-	new_offenders: BTreeSet<T::IdentificationTuple>,
 	/// Other reports for the same report kinds.
 	concurrent_offenders: Vec<OffenceDetails<T::AccountId, T::IdentificationTuple>>,
 }

srml/offences/src/mock.rs

@@ -46,6 +46,7 @@ impl<Reporter, Offender> offence::OnOffenceHandler<Reporter, Offender> for OnOff
 	fn on_offence(
 		_offenders: &[OffenceDetails<Reporter, Offender>],
 		slash_fraction: &[Perbill],
+		_offence_session: SessionIndex,
 	) {
 		ON_OFFENCE_PERBILL.with(|f| {
 			*f.borrow_mut() = slash_fraction.to_vec();
@@ -148,9 +149,7 @@ impl<T: Clone> offence::Offence<T> for Offence<T> {
 	}
 
 	fn session_index(&self) -> SessionIndex {
-		// session index is not used by the srml-offences directly, but rather it exists only for
-		// filtering historical reports.
-		unimplemented!()
+		1
 	}
 
 	fn slash_fraction(

srml/offences/src/tests.rs

@@ -23,6 +23,7 @@ use crate::mock::{
 	Offences, System, Offence, TestEvent, KIND, new_test_ext, with_on_offence_fractions,
 	offence_reports,
 };
+use sr_primitives::Perbill;
 use system::{EventRecord, Phase};
 
 #[test]
@@ -48,38 +49,6 @@ fn should_report_an_authority_and_trigger_on_offence() {
 	});
 }
 
-#[test]
-fn should_calculate_the_fraction_correctly() {
-	new_test_ext().execute_with(|| {
-		// given
-		let time_slot = 42;
-		assert_eq!(offence_reports(KIND, time_slot), vec![]);
-		let offence1 = Offence {
-			validator_set_count: 5,
-			time_slot,
-			offenders: vec![5],
-		};
-		let offence2 = Offence {
-			validator_set_count: 5,
-			time_slot,
-			offenders: vec![4],
-		};
-
-		// when
-		Offences::report_offence(vec![], offence1);
-		with_on_offence_fractions(|f| {
-			assert_eq!(f.clone(), vec![Perbill::from_percent(25)]);
-		});
-
-		Offences::report_offence(vec![], offence2);
-
-		// then
-		with_on_offence_fractions(|f| {
-			assert_eq!(f.clone(), vec![Perbill::from_percent(15), Perbill::from_percent(45)]);
-		});
-	});
-}
-
 #[test]
 fn should_not_report_the_same_authority_twice_in_the_same_slot() {
 	new_test_ext().execute_with(|| {