Offences report system rework (#13425)

* Experiments with common equivocation trait

* Improved equivocation trait

* Fix grandpa equivocation implementation

* Remove some cruft

* Remove some more cruft

* More generic naming

* Simplification of offences manipilation

* More refactory

* Some prograss with the encapsulation of offence report system

* Finally unit type works as a universal null report system

* Align substrate node code

* Further simplification

* Fix test utils

* Remove not required associated type

* Fix benches

* Rollback to prev field name

* Box big params

* Fix typo

* Remove new tag computation

* Remove default implementations

* Better docs

* Return 'Result' instead of bool

* Change offence report system return types

* Some renaming and documentation

* Improve documentation

* More abstract offence report system

* Rename 'consume_evidence' to 'process_evidence'

* Further docs refinements

* Doc for dummy offence report

* Fix rustdoc

* Fix after master merge

* Apply code review suggestions

* Improve docs

substrate/bin/node-template/runtime/src/lib.rs

@@ -214,21 +214,12 @@ impl pallet_aura::Config for Runtime {
 impl pallet_grandpa::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
 
-	type KeyOwnerProofSystem = ();
-
-	type KeyOwnerProof =
-		<Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(KeyTypeId, GrandpaId)>>::Proof;
-
-	type KeyOwnerIdentification = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(
-		KeyTypeId,
-		GrandpaId,
-	)>>::IdentificationTuple;
-
-	type HandleEquivocation = ();
-
 	type WeightInfo = ();
 	type MaxAuthorities = ConstU32<32>;
 	type MaxSetIdSessionEntries = ConstU64<0>;
+
+	type KeyOwnerProof = sp_core::Void;
+	type EquivocationReportSystem = ();
 }
 
 impl pallet_timestamp::Config for Runtime {

substrate/bin/node/runtime/src/lib.rs

@@ -113,6 +113,11 @@ pub mod assets_api;
 #[cfg(feature = "std")]
 include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs"));
 
+/// Max size for serialized extrinsic params for this testing runtime.
+/// This is a quite arbitrary but empirically battle tested value.
+#[cfg(test)]
+pub const CALL_PARAMS_MAX_SIZE: usize = 208;
+
 /// Wasm binary unwrapped. If built with `SKIP_WASM_BUILD`, the function panics.
 #[cfg(feature = "std")]
 pub fn wasm_binary_unwrap() -> &'static [u8] {
@@ -399,24 +404,12 @@ impl pallet_babe::Config for Runtime {
 	type ExpectedBlockTime = ExpectedBlockTime;
 	type EpochChangeTrigger = pallet_babe::ExternalTrigger;
 	type DisabledValidators = Session;
-
-	type KeyOwnerProofSystem = Historical;
-
-	type KeyOwnerProof = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(
-		KeyTypeId,
-		pallet_babe::AuthorityId,
-	)>>::Proof;
-
-	type KeyOwnerIdentification = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(
-		KeyTypeId,
-		pallet_babe::AuthorityId,
-	)>>::IdentificationTuple;
-
-	type HandleEquivocation =
-		pallet_babe::EquivocationHandler<Self::KeyOwnerIdentification, Offences, ReportLongevity>;
-
 	type WeightInfo = ();
 	type MaxAuthorities = MaxAuthorities;
+	type KeyOwnerProof =
+		<Historical as KeyOwnerProofSystem<(KeyTypeId, pallet_babe::AuthorityId)>>::Proof;
+	type EquivocationReportSystem =
+		pallet_babe::EquivocationReportSystem<Self, Offences, Historical, ReportLongevity>;
 }
 
 parameter_types! {
@@ -1328,26 +1321,12 @@ parameter_types! {
 
 impl pallet_grandpa::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
-
-	type KeyOwnerProofSystem = Historical;
-
-	type KeyOwnerProof =
-		<Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(KeyTypeId, GrandpaId)>>::Proof;
-
-	type KeyOwnerIdentification = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(
-		KeyTypeId,
-		GrandpaId,
-	)>>::IdentificationTuple;
-
-	type HandleEquivocation = pallet_grandpa::EquivocationHandler<
-		Self::KeyOwnerIdentification,
-		Offences,
-		ReportLongevity,
-	>;
-
 	type WeightInfo = ();
 	type MaxAuthorities = MaxAuthorities;
 	type MaxSetIdSessionEntries = MaxSetIdSessionEntries;
+	type KeyOwnerProof = <Historical as KeyOwnerProofSystem<(KeyTypeId, GrandpaId)>>::Proof;
+	type EquivocationReportSystem =
+		pallet_grandpa::EquivocationReportSystem<Self, Offences, Historical, ReportLongevity>;
 }
 
 parameter_types! {
@@ -2483,10 +2462,10 @@ mod tests {
 	fn call_size() {
 		let size = core::mem::size_of::<RuntimeCall>();
 		assert!(
-			size <= 208,
-			"size of RuntimeCall {} is more than 208 bytes: some calls have too big arguments, use Box to reduce the
-			size of RuntimeCall.
-			If the limit is too strong, maybe consider increase the limit to 300.",
+			size <= CALL_PARAMS_MAX_SIZE,
+			"size of RuntimeCall {} is more than {CALL_PARAMS_MAX_SIZE} bytes.
+			 Some calls have too big arguments, use Box to reduce the size of RuntimeCall.
+			 If the limit is too strong, maybe consider increase the limit.",
 			size,
 		);
 	}

substrate/frame/babe/src/equivocation.rs

@@ -34,143 +34,163 @@
 //! definition.
 
 use frame_support::traits::{Get, KeyOwnerProofSystem};
-use sp_consensus_babe::{EquivocationProof, Slot};
+use log::{error, info};
+
+use sp_consensus_babe::{AuthorityId, EquivocationProof, Slot, KEY_TYPE};
 use sp_runtime::{
 	transaction_validity::{
 		InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity,
 		TransactionValidityError, ValidTransaction,
 	},
-	DispatchResult, Perbill,
+	DispatchError, KeyTypeId, Perbill,
 };
+use sp_session::{GetSessionNumber, GetValidatorCount};
 use sp_staking::{
-	offence::{Kind, Offence, OffenceError, ReportOffence},
+	offence::{Kind, Offence, OffenceReportSystem, ReportOffence},
 	SessionIndex,
 };
 use sp_std::prelude::*;
 
-use crate::{Call, Config, Pallet, LOG_TARGET};
+use crate::{Call, Config, Error, Pallet, LOG_TARGET};
 
-/// A trait with utility methods for handling equivocation reports in BABE.
-/// The trait provides methods for reporting an offence triggered by a valid
-/// equivocation report, checking the current block author (to declare as the
-/// reporter), and also for creating and submitting equivocation report
-/// extrinsics (useful only in offchain context).
-pub trait HandleEquivocation<T: Config> {
-	/// The longevity, in blocks, that the equivocation report is valid for. When using the staking
-	/// pallet this should be equal to the bonding duration (in blocks, not eras).
-	type ReportLongevity: Get<u64>;
-
-	/// Report an offence proved by the given reporters.
-	fn report_offence(
-		reporters: Vec<T::AccountId>,
-		offence: BabeEquivocationOffence<T::KeyOwnerIdentification>,
-	) -> Result<(), OffenceError>;
-
-	/// Returns true if all of the offenders at the given time slot have already been reported.
-	fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &Slot) -> bool;
-
-	/// Create and dispatch an equivocation report extrinsic.
-	fn submit_unsigned_equivocation_report(
-		equivocation_proof: EquivocationProof<T::Header>,
-		key_owner_proof: T::KeyOwnerProof,
-	) -> DispatchResult;
-
-	/// Fetch the current block author id, if defined.
-	fn block_author() -> Option<T::AccountId>;
+/// BABE equivocation offence report.
+///
+/// When a validator released two or more blocks at the same slot.
+pub struct EquivocationOffence<Offender> {
+	/// A babe slot in which this incident happened.
+	pub slot: Slot,
+	/// The session index in which the incident happened.
+	pub session_index: SessionIndex,
+	/// The size of the validator set at the time of the offence.
+	pub validator_set_count: u32,
+	/// The authority that produced the equivocation.
+	pub offender: Offender,
 }
 
-impl<T: Config> HandleEquivocation<T> for () {
-	type ReportLongevity = ();
+impl<Offender: Clone> Offence<Offender> for EquivocationOffence<Offender> {
+	const ID: Kind = *b"babe:equivocatio";
+	type TimeSlot = Slot;
 
-	fn report_offence(
-		_reporters: Vec<T::AccountId>,
-		_offence: BabeEquivocationOffence<T::KeyOwnerIdentification>,
-	) -> Result<(), OffenceError> {
-		Ok(())
+	fn offenders(&self) -> Vec<Offender> {
+		vec![self.offender.clone()]
 	}
 
-	fn is_known_offence(_offenders: &[T::KeyOwnerIdentification], _time_slot: &Slot) -> bool {
-		true
+	fn session_index(&self) -> SessionIndex {
+		self.session_index
 	}
 
-	fn submit_unsigned_equivocation_report(
-		_equivocation_proof: EquivocationProof<T::Header>,
-		_key_owner_proof: T::KeyOwnerProof,
-	) -> DispatchResult {
-		Ok(())
+	fn validator_set_count(&self) -> u32 {
+		self.validator_set_count
 	}
 
-	fn block_author() -> Option<T::AccountId> {
-		None
+	fn time_slot(&self) -> Self::TimeSlot {
+		self.slot
+	}
+
+	// The formula is min((3k / n)^2, 1)
+	// where k = offenders_number and n = validators_number
+	fn slash_fraction(&self, offenders_count: u32) -> Perbill {
+		// Perbill type domain is [0, 1] by definition
+		Perbill::from_rational(3 * offenders_count, self.validator_set_count).square()
 	}
 }
 
-/// Generic equivocation handler. This type implements `HandleEquivocation`
-/// using existing subsystems that are part of frame (type bounds described
-/// below) and will dispatch to them directly, it's only purpose is to wire all
-/// subsystems together.
-pub struct EquivocationHandler<I, R, L> {
-	_phantom: sp_std::marker::PhantomData<(I, R, L)>,
-}
+/// Babe equivocation offence system.
+///
+/// This type implements `OffenceReportSystem`
+pub struct EquivocationReportSystem<T, R, P, L>(sp_std::marker::PhantomData<(T, R, P, L)>);
 
-impl<I, R, L> Default for EquivocationHandler<I, R, L> {
-	fn default() -> Self {
-		Self { _phantom: Default::default() }
-	}
-}
-
-impl<T, R, L> HandleEquivocation<T> for EquivocationHandler<T::KeyOwnerIdentification, R, L>
+// We use the authorship pallet to fetch the current block author and use
+// `offchain::SendTransactionTypes` for unsigned extrinsic creation and
+// submission.
+impl<T, R, P, L>
+	OffenceReportSystem<Option<T::AccountId>, (EquivocationProof<T::Header>, T::KeyOwnerProof)>
+	for EquivocationReportSystem<T, R, P, L>
 where
-	// We use the authorship pallet to fetch the current block author and use
-	// `offchain::SendTransactionTypes` for unsigned extrinsic creation and
-	// submission.
 	T: Config + pallet_authorship::Config + frame_system::offchain::SendTransactionTypes<Call<T>>,
-	// A system for reporting offences after valid equivocation reports are
-	// processed.
 	R: ReportOffence<
 		T::AccountId,
-		T::KeyOwnerIdentification,
-		BabeEquivocationOffence<T::KeyOwnerIdentification>,
+		P::IdentificationTuple,
+		EquivocationOffence<P::IdentificationTuple>,
 	>,
-	// The longevity (in blocks) that the equivocation report is valid for. When using the staking
-	// pallet this should be the bonding duration.
+	P: KeyOwnerProofSystem<(KeyTypeId, AuthorityId), Proof = T::KeyOwnerProof>,
+	P::IdentificationTuple: Clone,
 	L: Get<u64>,
 {
-	type ReportLongevity = L;
+	type Longevity = L;
 
-	fn report_offence(
-		reporters: Vec<T::AccountId>,
-		offence: BabeEquivocationOffence<T::KeyOwnerIdentification>,
-	) -> Result<(), OffenceError> {
-		R::report_offence(reporters, offence)
-	}
-
-	fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &Slot) -> bool {
-		R::is_known_offence(offenders, time_slot)
-	}
-
-	fn submit_unsigned_equivocation_report(
-		equivocation_proof: EquivocationProof<T::Header>,
-		key_owner_proof: T::KeyOwnerProof,
-	) -> DispatchResult {
+	fn publish_evidence(
+		evidence: (EquivocationProof<T::Header>, T::KeyOwnerProof),
+	) -> Result<(), ()> {
 		use frame_system::offchain::SubmitTransaction;
+		let (equivocation_proof, key_owner_proof) = evidence;
 
 		let call = Call::report_equivocation_unsigned {
 			equivocation_proof: Box::new(equivocation_proof),
 			key_owner_proof,
 		};
-
-		match SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into()) {
-			Ok(()) => log::info!(target: LOG_TARGET, "Submitted BABE equivocation report.",),
-			Err(e) =>
-				log::error!(target: LOG_TARGET, "Error submitting equivocation report: {:?}", e,),
+		let res = SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into());
+		match res {
+			Ok(()) => info!(target: LOG_TARGET, "Submitted equivocation report."),
+			Err(e) => error!(target: LOG_TARGET, "Error submitting equivocation report: {:?}", e),
 		}
-
-		Ok(())
+		res
 	}
 
-	fn block_author() -> Option<T::AccountId> {
-		<pallet_authorship::Pallet<T>>::author()
+	fn check_evidence(
+		evidence: (EquivocationProof<T::Header>, T::KeyOwnerProof),
+	) -> Result<(), TransactionValidityError> {
+		let (equivocation_proof, key_owner_proof) = evidence;
+
+		// Check the membership proof to extract the offender's id
+		let key = (sp_consensus_babe::KEY_TYPE, equivocation_proof.offender.clone());
+		let offender =
+			P::check_proof(key, key_owner_proof.clone()).ok_or(InvalidTransaction::BadProof)?;
+
+		// Check if the offence has already been reported, and if so then we can discard the report.
+		if R::is_known_offence(&[offender], &equivocation_proof.slot) {
+			Err(InvalidTransaction::Stale.into())
+		} else {
+			Ok(())
+		}
+	}
+
+	fn process_evidence(
+		reporter: Option<T::AccountId>,
+		evidence: (EquivocationProof<T::Header>, T::KeyOwnerProof),
+	) -> Result<(), DispatchError> {
+		let (equivocation_proof, key_owner_proof) = evidence;
+		let reporter = reporter.or_else(|| <pallet_authorship::Pallet<T>>::author());
+		let offender = equivocation_proof.offender.clone();
+		let slot = equivocation_proof.slot;
+
+		// Validate the equivocation proof (check votes are different and signatures are valid)
+		if !sp_consensus_babe::check_equivocation_proof(equivocation_proof) {
+			return Err(Error::<T>::InvalidEquivocationProof.into())
+		}
+
+		let validator_set_count = key_owner_proof.validator_count();
+		let session_index = key_owner_proof.session();
+
+		let epoch_index =
+			*slot.saturating_sub(crate::GenesisSlot::<T>::get()) / T::EpochDuration::get();
+
+		// Check that the slot number is consistent with the session index
+		// in the key ownership proof (i.e. slot is for that epoch)
+		if Pallet::<T>::session_index_for_epoch(epoch_index) != session_index {
+			return Err(Error::<T>::InvalidKeyOwnershipProof.into())
+		}
+
+		// Check the membership proof and extract the offender's id
+		let offender = P::check_proof((KEY_TYPE, offender), key_owner_proof)
+			.ok_or(Error::<T>::InvalidKeyOwnershipProof)?;
+
+		let offence = EquivocationOffence { slot, validator_set_count, offender, session_index };
+
+		R::report_offence(reporter.into_iter().collect(), offence)
+			.map_err(|_| Error::<T>::DuplicateOffenceReport)?;
+
+		Ok(())
 	}
 }
 
@@ -194,11 +214,12 @@ impl<T: Config> Pallet<T> {
 				},
 			}
 
-			// check report staleness
-			is_known_offence::<T>(equivocation_proof, key_owner_proof)?;
+			// Check report validity
+			let evidence = (*equivocation_proof.clone(), key_owner_proof.clone());
+			T::EquivocationReportSystem::check_evidence(evidence)?;
 
 			let longevity =
-				<T::HandleEquivocation as HandleEquivocation<T>>::ReportLongevity::get();
+				<T::EquivocationReportSystem as OffenceReportSystem<_, _>>::Longevity::get();
 
 			ValidTransaction::with_tag_prefix("BabeEquivocation")
 				// We assign the maximum priority for any equivocation report.
@@ -216,72 +237,10 @@ impl<T: Config> Pallet<T> {
 
 	pub fn pre_dispatch(call: &Call<T>) -> Result<(), TransactionValidityError> {
 		if let Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof } = call {
-			is_known_offence::<T>(equivocation_proof, key_owner_proof)
+			let evidence = (*equivocation_proof.clone(), key_owner_proof.clone());
+			T::EquivocationReportSystem::check_evidence(evidence)
 		} else {
 			Err(InvalidTransaction::Call.into())
 		}
 	}
 }
-
-fn is_known_offence<T: Config>(
-	equivocation_proof: &EquivocationProof<T::Header>,
-	key_owner_proof: &T::KeyOwnerProof,
-) -> Result<(), TransactionValidityError> {
-	// check the membership proof to extract the offender's id
-	let key = (sp_consensus_babe::KEY_TYPE, equivocation_proof.offender.clone());
-
-	let offender = T::KeyOwnerProofSystem::check_proof(key, key_owner_proof.clone())
-		.ok_or(InvalidTransaction::BadProof)?;
-
-	// check if the offence has already been reported,
-	// and if so then we can discard the report.
-	if T::HandleEquivocation::is_known_offence(&[offender], &equivocation_proof.slot) {
-		Err(InvalidTransaction::Stale.into())
-	} else {
-		Ok(())
-	}
-}
-
-/// A BABE equivocation offence report.
-///
-/// When a validator released two or more blocks at the same slot.
-pub struct BabeEquivocationOffence<FullIdentification> {
-	/// A babe slot in which this incident happened.
-	pub slot: Slot,
-	/// The session index in which the incident happened.
-	pub session_index: SessionIndex,
-	/// The size of the validator set at the time of the offence.
-	pub validator_set_count: u32,
-	/// The authority that produced the equivocation.
-	pub offender: FullIdentification,
-}
-
-impl<FullIdentification: Clone> Offence<FullIdentification>
-	for BabeEquivocationOffence<FullIdentification>
-{
-	const ID: Kind = *b"babe:equivocatio";
-	type TimeSlot = Slot;
-
-	fn offenders(&self) -> Vec<FullIdentification> {
-		vec![self.offender.clone()]
-	}
-
-	fn session_index(&self) -> SessionIndex {
-		self.session_index
-	}
-
-	fn validator_set_count(&self) -> u32 {
-		self.validator_set_count
-	}
-
-	fn time_slot(&self) -> Self::TimeSlot {
-		self.slot
-	}
-
-	fn slash_fraction(&self, offenders_count: u32) -> Perbill {
-		// the formula is min((3k / n)^2, 1)
-		let x = Perbill::from_rational(3 * offenders_count, self.validator_set_count);
-		// _ ^ 2
-		x.square()
-	}
-}

substrate/frame/babe/src/lib.rs

@@ -25,29 +25,25 @@ use codec::{Decode, Encode};
 use frame_support::{
 	dispatch::{DispatchResultWithPostInfo, Pays},
 	ensure,
-	traits::{
-		ConstU32, DisabledValidators, FindAuthor, Get, KeyOwnerProofSystem, OnTimestampSet,
-		OneSessionHandler,
-	},
+	traits::{ConstU32, DisabledValidators, FindAuthor, Get, OnTimestampSet, OneSessionHandler},
 	weights::Weight,
 	BoundedVec, WeakBoundedVec,
 };
 use sp_application_crypto::ByteArray;
-use sp_runtime::{
-	generic::DigestItem,
-	traits::{IsMember, One, SaturatedConversion, Saturating, Zero},
-	ConsensusEngineId, KeyTypeId, Permill,
-};
-use sp_session::{GetSessionNumber, GetValidatorCount};
-use sp_staking::SessionIndex;
-use sp_std::prelude::*;
-
 use sp_consensus_babe::{
 	digests::{NextConfigDescriptor, NextEpochDescriptor, PreDigest},
 	AllowedSlots, BabeAuthorityWeight, BabeEpochConfiguration, ConsensusLog, Epoch,
 	EquivocationProof, Slot, BABE_ENGINE_ID,
 };
 use sp_consensus_vrf::schnorrkel;
+use sp_runtime::{
+	generic::DigestItem,
+	traits::{IsMember, One, SaturatedConversion, Saturating, Zero},
+	ConsensusEngineId, Permill,
+};
+use sp_session::{GetSessionNumber, GetValidatorCount};
+use sp_staking::{offence::OffenceReportSystem, SessionIndex};
+use sp_std::prelude::*;
 
 pub use sp_consensus_babe::{AuthorityId, PUBLIC_KEY_LENGTH, RANDOMNESS_LENGTH, VRF_OUTPUT_LENGTH};
 
@@ -64,7 +60,7 @@ mod mock;
 #[cfg(all(feature = "std", test))]
 mod tests;
 
-pub use equivocation::{BabeEquivocationOffence, EquivocationHandler, HandleEquivocation};
+pub use equivocation::{EquivocationOffence, EquivocationReportSystem};
 #[allow(deprecated)]
 pub use randomness::CurrentBlockRandomness;
 pub use randomness::{
@@ -150,35 +146,25 @@ pub mod pallet {
 		/// initialization.
 		type DisabledValidators: DisabledValidators;
 
-		/// The proof of key ownership, used for validating equivocation reports.
-		/// The proof must include the session index and validator count of the
-		/// session at which the equivocation occurred.
-		type KeyOwnerProof: Parameter + GetSessionNumber + GetValidatorCount;
-
-		/// The identification of a key owner, used when reporting equivocations.
-		type KeyOwnerIdentification: Parameter;
-
-		/// A system for proving ownership of keys, i.e. that a given key was part
-		/// of a validator set, needed for validating equivocation reports.
-		type KeyOwnerProofSystem: KeyOwnerProofSystem<
-			(KeyTypeId, AuthorityId),
-			Proof = Self::KeyOwnerProof,
-			IdentificationTuple = Self::KeyOwnerIdentification,
-		>;
-
-		/// The equivocation handling subsystem, defines methods to report an
-		/// offence (after the equivocation has been validated) and for submitting a
-		/// transaction to report an equivocation (from an offchain context).
-		/// NOTE: when enabling equivocation handling (i.e. this type isn't set to
-		/// `()`) you must use this pallet's `ValidateUnsigned` in the runtime
-		/// definition.
-		type HandleEquivocation: HandleEquivocation<Self>;
-
+		/// Helper for weights computations
 		type WeightInfo: WeightInfo;
 
 		/// Max number of authorities allowed
 		#[pallet::constant]
 		type MaxAuthorities: Get<u32>;
+
+		/// The proof of key ownership, used for validating equivocation reports.
+		/// The proof must include the session index and validator count of the
+		/// session at which the equivocation occurred.
+		type KeyOwnerProof: Parameter + GetSessionNumber + GetValidatorCount;
+
+		/// The equivocation handling subsystem, defines methods to check/report an
+		/// offence and for submitting a transaction to report an equivocation
+		/// (from an offchain context).
+		type EquivocationReportSystem: OffenceReportSystem<
+			Option<Self::AccountId>,
+			(EquivocationProof<Self::Header>, Self::KeyOwnerProof),
+		>;
 	}
 
 	#[pallet::error]
@@ -429,8 +415,12 @@ pub mod pallet {
 			key_owner_proof: T::KeyOwnerProof,
 		) -> DispatchResultWithPostInfo {
 			let reporter = ensure_signed(origin)?;
-
-			Self::do_report_equivocation(Some(reporter), *equivocation_proof, key_owner_proof)
+			T::EquivocationReportSystem::process_evidence(
+				Some(reporter),
+				(*equivocation_proof, key_owner_proof),
+			)?;
+			// Waive the fee since the report is valid and beneficial
+			Ok(Pays::No.into())
 		}
 
 		/// Report authority equivocation/misbehavior. This method will verify
@@ -451,12 +441,11 @@ pub mod pallet {
 			key_owner_proof: T::KeyOwnerProof,
 		) -> DispatchResultWithPostInfo {
 			ensure_none(origin)?;
-
-			Self::do_report_equivocation(
-				T::HandleEquivocation::block_author(),
-				*equivocation_proof,
-				key_owner_proof,
-			)
+			T::EquivocationReportSystem::process_evidence(
+				None,
+				(*equivocation_proof, key_owner_proof),
+			)?;
+			Ok(Pays::No.into())
 		}
 
 		/// Plan an epoch config change. The epoch config change is recorded and will be enacted on
@@ -866,7 +855,7 @@ impl<T: Config> Pallet<T> {
 	/// This function is only well defined for epochs that actually existed,
 	/// e.g. if we skipped from epoch 10 to 20 then a call for epoch 15 (which
 	/// didn't exist) will return an incorrect session index.
-	fn session_index_for_epoch(epoch_index: u64) -> SessionIndex {
+	pub(crate) fn session_index_for_epoch(epoch_index: u64) -> SessionIndex {
 		let skipped_epochs = SkippedEpochs::<T>::get();
 		match skipped_epochs.binary_search_by_key(&epoch_index, |(epoch_index, _)| *epoch_index) {
 			// we have an exact match so we just return the given session index
@@ -890,50 +879,6 @@ impl<T: Config> Pallet<T> {
 		}
 	}
 
-	fn do_report_equivocation(
-		reporter: Option<T::AccountId>,
-		equivocation_proof: EquivocationProof<T::Header>,
-		key_owner_proof: T::KeyOwnerProof,
-	) -> DispatchResultWithPostInfo {
-		let offender = equivocation_proof.offender.clone();
-		let slot = equivocation_proof.slot;
-
-		// validate the equivocation proof
-		if !sp_consensus_babe::check_equivocation_proof(equivocation_proof) {
-			return Err(Error::<T>::InvalidEquivocationProof.into())
-		}
-
-		let validator_set_count = key_owner_proof.validator_count();
-		let session_index = key_owner_proof.session();
-
-		let epoch_index = *slot.saturating_sub(GenesisSlot::<T>::get()) / T::EpochDuration::get();
-
-		// check that the slot number is consistent with the session index
-		// in the key ownership proof (i.e. slot is for that epoch)
-		if Self::session_index_for_epoch(epoch_index) != session_index {
-			return Err(Error::<T>::InvalidKeyOwnershipProof.into())
-		}
-
-		// check the membership proof and extract the offender's id
-		let key = (sp_consensus_babe::KEY_TYPE, offender);
-		let offender = T::KeyOwnerProofSystem::check_proof(key, key_owner_proof)
-			.ok_or(Error::<T>::InvalidKeyOwnershipProof)?;
-
-		let offence =
-			BabeEquivocationOffence { slot, validator_set_count, offender, session_index };
-
-		let reporters = match reporter {
-			Some(id) => vec![id],
-			None => vec![],
-		};
-
-		T::HandleEquivocation::report_offence(reporters, offence)
-			.map_err(|_| Error::<T>::DuplicateOffenceReport)?;
-
-		// waive the fee since the report is valid and beneficial
-		Ok(Pays::No.into())
-	}
-
 	/// Submits an extrinsic to report an equivocation. This method will create
 	/// an unsigned extrinsic with a call to `report_equivocation_unsigned` and
 	/// will push the transaction to the pool. Only useful in an offchain
@@ -942,11 +887,7 @@ impl<T: Config> Pallet<T> {
 		equivocation_proof: EquivocationProof<T::Header>,
 		key_owner_proof: T::KeyOwnerProof,
 	) -> Option<()> {
-		T::HandleEquivocation::submit_unsigned_equivocation_report(
-			equivocation_proof,
-			key_owner_proof,
-		)
-		.ok()
+		T::EquivocationReportSystem::publish_evidence((equivocation_proof, key_owner_proof)).ok()
 	}
 }
 

substrate/frame/babe/src/mock.rs

@@ -223,22 +223,11 @@ impl Config for Test {
 	type ExpectedBlockTime = ConstU64<1>;
 	type EpochChangeTrigger = crate::ExternalTrigger;
 	type DisabledValidators = Session;
-
-	type KeyOwnerProofSystem = Historical;
-
-	type KeyOwnerProof =
-		<Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(KeyTypeId, AuthorityId)>>::Proof;
-
-	type KeyOwnerIdentification = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(
-		KeyTypeId,
-		AuthorityId,
-	)>>::IdentificationTuple;
-
-	type HandleEquivocation =
-		super::EquivocationHandler<Self::KeyOwnerIdentification, Offences, ReportLongevity>;
-
 	type WeightInfo = ();
 	type MaxAuthorities = ConstU32<10>;
+	type KeyOwnerProof = <Historical as KeyOwnerProofSystem<(KeyTypeId, AuthorityId)>>::Proof;
+	type EquivocationReportSystem =
+		super::EquivocationReportSystem<Self, Offences, Historical, ReportLongevity>;
 }
 
 pub fn go_to_block(n: u64, s: u64) {

substrate/frame/babe/src/tests.rs

@@ -21,7 +21,7 @@ use super::{Call, *};
 use frame_support::{
 	assert_err, assert_noop, assert_ok,
 	dispatch::{GetDispatchInfo, Pays},
-	traits::{Currency, EstimateNextSessionRotation, OnFinalize},
+	traits::{Currency, EstimateNextSessionRotation, KeyOwnerProofSystem, OnFinalize},
 };
 use mock::*;
 use pallet_session::ShouldEndSession;

substrate/frame/grandpa/src/equivocation.rs

@@ -35,153 +35,25 @@
 //! that the `ValidateUnsigned` for the GRANDPA pallet is used in the runtime
 //! definition.
 
-use sp_std::prelude::*;
-
 use codec::{self as codec, Decode, Encode};
 use frame_support::traits::{Get, KeyOwnerProofSystem};
-use sp_consensus_grandpa::{EquivocationProof, RoundNumber, SetId};
+use log::{error, info};
+use sp_consensus_grandpa::{AuthorityId, EquivocationProof, RoundNumber, SetId, KEY_TYPE};
 use sp_runtime::{
 	transaction_validity::{
 		InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity,
 		TransactionValidityError, ValidTransaction,
 	},
-	DispatchResult, Perbill,
+	DispatchError, KeyTypeId, Perbill,
 };
+use sp_session::{GetSessionNumber, GetValidatorCount};
 use sp_staking::{
-	offence::{Kind, Offence, OffenceError, ReportOffence},
+	offence::{Kind, Offence, OffenceReportSystem, ReportOffence},
 	SessionIndex,
 };
+use sp_std::prelude::*;
 
-use super::{Call, Config, Pallet, LOG_TARGET};
-
-/// A trait with utility methods for handling equivocation reports in GRANDPA.
-/// The offence type is generic, and the trait provides , reporting an offence
-/// triggered by a valid equivocation report, and also for creating and
-/// submitting equivocation report extrinsics (useful only in offchain context).
-pub trait HandleEquivocation<T: Config> {
-	/// The offence type used for reporting offences on valid equivocation reports.
-	type Offence: GrandpaOffence<T::KeyOwnerIdentification>;
-
-	/// The longevity, in blocks, that the equivocation report is valid for. When using the staking
-	/// pallet this should be equal to the bonding duration (in blocks, not eras).
-	type ReportLongevity: Get<u64>;
-
-	/// Report an offence proved by the given reporters.
-	fn report_offence(
-		reporters: Vec<T::AccountId>,
-		offence: Self::Offence,
-	) -> Result<(), OffenceError>;
-
-	/// Returns true if all of the offenders at the given time slot have already been reported.
-	fn is_known_offence(
-		offenders: &[T::KeyOwnerIdentification],
-		time_slot: &<Self::Offence as Offence<T::KeyOwnerIdentification>>::TimeSlot,
-	) -> bool;
-
-	/// Create and dispatch an equivocation report extrinsic.
-	fn submit_unsigned_equivocation_report(
-		equivocation_proof: EquivocationProof<T::Hash, T::BlockNumber>,
-		key_owner_proof: T::KeyOwnerProof,
-	) -> DispatchResult;
-
-	/// Fetch the current block author id, if defined.
-	fn block_author() -> Option<T::AccountId>;
-}
-
-impl<T: Config> HandleEquivocation<T> for () {
-	type Offence = GrandpaEquivocationOffence<T::KeyOwnerIdentification>;
-	type ReportLongevity = ();
-
-	fn report_offence(
-		_reporters: Vec<T::AccountId>,
-		_offence: GrandpaEquivocationOffence<T::KeyOwnerIdentification>,
-	) -> Result<(), OffenceError> {
-		Ok(())
-	}
-
-	fn is_known_offence(
-		_offenders: &[T::KeyOwnerIdentification],
-		_time_slot: &GrandpaTimeSlot,
-	) -> bool {
-		true
-	}
-
-	fn submit_unsigned_equivocation_report(
-		_equivocation_proof: EquivocationProof<T::Hash, T::BlockNumber>,
-		_key_owner_proof: T::KeyOwnerProof,
-	) -> DispatchResult {
-		Ok(())
-	}
-
-	fn block_author() -> Option<T::AccountId> {
-		None
-	}
-}
-
-/// Generic equivocation handler. This type implements `HandleEquivocation`
-/// using existing subsystems that are part of frame (type bounds described
-/// below) and will dispatch to them directly, it's only purpose is to wire all
-/// subsystems together.
-pub struct EquivocationHandler<I, R, L, O = GrandpaEquivocationOffence<I>> {
-	_phantom: sp_std::marker::PhantomData<(I, R, L, O)>,
-}
-
-impl<I, R, L, O> Default for EquivocationHandler<I, R, L, O> {
-	fn default() -> Self {
-		Self { _phantom: Default::default() }
-	}
-}
-
-impl<T, R, L, O> HandleEquivocation<T> for EquivocationHandler<T::KeyOwnerIdentification, R, L, O>
-where
-	// We use the authorship pallet to fetch the current block author and use
-	// `offchain::SendTransactionTypes` for unsigned extrinsic creation and
-	// submission.
-	T: Config + pallet_authorship::Config + frame_system::offchain::SendTransactionTypes<Call<T>>,
-	// A system for reporting offences after valid equivocation reports are
-	// processed.
-	R: ReportOffence<T::AccountId, T::KeyOwnerIdentification, O>,
-	// The longevity (in blocks) that the equivocation report is valid for. When using the staking
-	// pallet this should be the bonding duration.
-	L: Get<u64>,
-	// The offence type that should be used when reporting.
-	O: GrandpaOffence<T::KeyOwnerIdentification>,
-{
-	type Offence = O;
-	type ReportLongevity = L;
-
-	fn report_offence(reporters: Vec<T::AccountId>, offence: O) -> Result<(), OffenceError> {
-		R::report_offence(reporters, offence)
-	}
-
-	fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &O::TimeSlot) -> bool {
-		R::is_known_offence(offenders, time_slot)
-	}
-
-	fn submit_unsigned_equivocation_report(
-		equivocation_proof: EquivocationProof<T::Hash, T::BlockNumber>,
-		key_owner_proof: T::KeyOwnerProof,
-	) -> DispatchResult {
-		use frame_system::offchain::SubmitTransaction;
-
-		let call = Call::report_equivocation_unsigned {
-			equivocation_proof: Box::new(equivocation_proof),
-			key_owner_proof,
-		};
-
-		match SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into()) {
-			Ok(()) => log::info!(target: LOG_TARGET, "Submitted GRANDPA equivocation report.",),
-			Err(e) =>
-				log::error!(target: LOG_TARGET, "Error submitting equivocation report: {:?}", e,),
-		}
-
-		Ok(())
-	}
-
-	fn block_author() -> Option<T::AccountId> {
-		<pallet_authorship::Pallet<T>>::author()
-	}
-}
+use super::{Call, Config, Error, Pallet, LOG_TARGET};
 
 /// A round number and set id which point on the time of an offence.
 #[derive(Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Encode, Decode)]
@@ -193,6 +65,175 @@ pub struct GrandpaTimeSlot {
 	pub round: RoundNumber,
 }
 
+/// A GRANDPA equivocation offence report.
+pub struct EquivocationOffence<Offender> {
+	/// Time slot at which this incident happened.
+	pub time_slot: GrandpaTimeSlot,
+	/// The session index in which the incident happened.
+	pub session_index: SessionIndex,
+	/// The size of the validator set at the time of the offence.
+	pub validator_set_count: u32,
+	/// The authority which produced this equivocation.
+	pub offender: Offender,
+}
+
+impl<Offender: Clone> Offence<Offender> for EquivocationOffence<Offender> {
+	const ID: Kind = *b"grandpa:equivoca";
+	type TimeSlot = GrandpaTimeSlot;
+
+	fn offenders(&self) -> Vec<Offender> {
+		vec![self.offender.clone()]
+	}
+
+	fn session_index(&self) -> SessionIndex {
+		self.session_index
+	}
+
+	fn validator_set_count(&self) -> u32 {
+		self.validator_set_count
+	}
+
+	fn time_slot(&self) -> Self::TimeSlot {
+		self.time_slot
+	}
+
+	// The formula is min((3k / n)^2, 1)
+	// where k = offenders_number and n = validators_number
+	fn slash_fraction(&self, offenders_count: u32) -> Perbill {
+		// Perbill type domain is [0, 1] by definition
+		Perbill::from_rational(3 * offenders_count, self.validator_set_count).square()
+	}
+}
+
+/// Generic equivocation handler. This type implements `HandleEquivocation`
+/// using existing subsystems that are part of frame (type bounds described
+/// below) and will dispatch to them directly, it's only purpose is to wire all
+/// subsystems together.
+pub struct EquivocationReportSystem<T, R, P, L>(sp_std::marker::PhantomData<(T, R, P, L)>);
+
+// We use the authorship pallet to fetch the current block author and use
+// `offchain::SendTransactionTypes` for unsigned extrinsic creation and
+// submission.
+impl<T, R, P, L>
+	OffenceReportSystem<
+		Option<T::AccountId>,
+		(EquivocationProof<T::Hash, T::BlockNumber>, T::KeyOwnerProof),
+	> for EquivocationReportSystem<T, R, P, L>
+where
+	T: Config + pallet_authorship::Config + frame_system::offchain::SendTransactionTypes<Call<T>>,
+	R: ReportOffence<
+		T::AccountId,
+		P::IdentificationTuple,
+		EquivocationOffence<P::IdentificationTuple>,
+	>,
+	P: KeyOwnerProofSystem<(KeyTypeId, AuthorityId), Proof = T::KeyOwnerProof>,
+	P::IdentificationTuple: Clone,
+	L: Get<u64>,
+{
+	type Longevity = L;
+
+	fn publish_evidence(
+		evidence: (EquivocationProof<T::Hash, T::BlockNumber>, T::KeyOwnerProof),
+	) -> Result<(), ()> {
+		use frame_system::offchain::SubmitTransaction;
+		let (equivocation_proof, key_owner_proof) = evidence;
+
+		let call = Call::report_equivocation_unsigned {
+			equivocation_proof: Box::new(equivocation_proof),
+			key_owner_proof,
+		};
+		let res = SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into());
+		match res {
+			Ok(()) => info!(target: LOG_TARGET, "Submitted equivocation report."),
+			Err(e) => error!(target: LOG_TARGET, "Error submitting equivocation report: {:?}", e),
+		}
+		res
+	}
+
+	fn check_evidence(
+		evidence: (EquivocationProof<T::Hash, T::BlockNumber>, T::KeyOwnerProof),
+	) -> Result<(), TransactionValidityError> {
+		let (equivocation_proof, key_owner_proof) = evidence;
+
+		// Check the membership proof to extract the offender's id
+		let key = (KEY_TYPE, equivocation_proof.offender().clone());
+		let offender = P::check_proof(key, key_owner_proof).ok_or(InvalidTransaction::BadProof)?;
+
+		// Check if the offence has already been reported, and if so then we can discard the report.
+		let time_slot = GrandpaTimeSlot {
+			set_id: equivocation_proof.set_id(),
+			round: equivocation_proof.round(),
+		};
+		if R::is_known_offence(&[offender], &time_slot) {
+			Err(InvalidTransaction::Stale.into())
+		} else {
+			Ok(())
+		}
+	}
+
+	fn process_evidence(
+		reporter: Option<T::AccountId>,
+		evidence: (EquivocationProof<T::Hash, T::BlockNumber>, T::KeyOwnerProof),
+	) -> Result<(), DispatchError> {
+		let (equivocation_proof, key_owner_proof) = evidence;
+		let reporter = reporter.or_else(|| <pallet_authorship::Pallet<T>>::author());
+		let offender = equivocation_proof.offender().clone();
+
+		// We check the equivocation within the context of its set id (and
+		// associated session) and round. We also need to know the validator
+		// set count when the offence since it is required to calculate the
+		// slash amount.
+		let set_id = equivocation_proof.set_id();
+		let round = equivocation_proof.round();
+		let session_index = key_owner_proof.session();
+		let validator_set_count = key_owner_proof.validator_count();
+
+		// Validate equivocation proof (check votes are different and signatures are valid).
+		if !sp_consensus_grandpa::check_equivocation_proof(equivocation_proof) {
+			return Err(Error::<T>::InvalidEquivocationProof.into())
+		}
+
+		// Validate the key ownership proof extracting the id of the offender.
+		let offender = P::check_proof((KEY_TYPE, offender), key_owner_proof)
+			.ok_or(Error::<T>::InvalidKeyOwnershipProof)?;
+
+		// Fetch the current and previous sets last session index.
+		// For genesis set there's no previous set.
+		let previous_set_id_session_index = if set_id != 0 {
+			let idx = crate::SetIdSession::<T>::get(set_id - 1)
+				.ok_or(Error::<T>::InvalidEquivocationProof)?;
+			Some(idx)
+		} else {
+			None
+		};
+
+		let set_id_session_index =
+			crate::SetIdSession::<T>::get(set_id).ok_or(Error::<T>::InvalidEquivocationProof)?;
+
+		// Check that the session id for the membership proof is within the
+		// bounds of the set id reported in the equivocation.
+		if session_index > set_id_session_index ||
+			previous_set_id_session_index
+				.map(|previous_index| session_index <= previous_index)
+				.unwrap_or(false)
+		{
+			return Err(Error::<T>::InvalidEquivocationProof.into())
+		}
+
+		let offence = EquivocationOffence {
+			time_slot: GrandpaTimeSlot { set_id, round },
+			session_index,
+			offender,
+			validator_set_count,
+		};
+
+		R::report_offence(reporter.into_iter().collect(), offence)
+			.map_err(|_| Error::<T>::DuplicateOffenceReport)?;
+
+		Ok(())
+	}
+}
+
 /// Methods for the `ValidateUnsigned` implementation:
 /// It restricts calls to `report_equivocation_unsigned` to local calls (i.e. extrinsics generated
 /// on this node) or that already in a block. This guarantees that only block authors can include
@@ -213,11 +254,12 @@ impl<T: Config> Pallet<T> {
 				},
 			}
 
-			// check report staleness
-			is_known_offence::<T>(equivocation_proof, key_owner_proof)?;
+			// Check report validity
+			let evidence = (*equivocation_proof.clone(), key_owner_proof.clone());
+			T::EquivocationReportSystem::check_evidence(evidence)?;
 
 			let longevity =
-				<T::HandleEquivocation as HandleEquivocation<T>>::ReportLongevity::get();
+				<T::EquivocationReportSystem as OffenceReportSystem<_, _>>::Longevity::get();
 
 			ValidTransaction::with_tag_prefix("GrandpaEquivocation")
 				// We assign the maximum priority for any equivocation report.
@@ -239,118 +281,10 @@ impl<T: Config> Pallet<T> {
 
 	pub fn pre_dispatch(call: &Call<T>) -> Result<(), TransactionValidityError> {
 		if let Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof } = call {
-			is_known_offence::<T>(equivocation_proof, key_owner_proof)
+			let evidence = (*equivocation_proof.clone(), key_owner_proof.clone());
+			T::EquivocationReportSystem::check_evidence(evidence)
 		} else {
 			Err(InvalidTransaction::Call.into())
 		}
 	}
 }
-
-fn is_known_offence<T: Config>(
-	equivocation_proof: &EquivocationProof<T::Hash, T::BlockNumber>,
-	key_owner_proof: &T::KeyOwnerProof,
-) -> Result<(), TransactionValidityError> {
-	// check the membership proof to extract the offender's id
-	let key = (sp_consensus_grandpa::KEY_TYPE, equivocation_proof.offender().clone());
-
-	let offender = T::KeyOwnerProofSystem::check_proof(key, key_owner_proof.clone())
-		.ok_or(InvalidTransaction::BadProof)?;
-
-	// check if the offence has already been reported,
-	// and if so then we can discard the report.
-	let time_slot = <T::HandleEquivocation as HandleEquivocation<T>>::Offence::new_time_slot(
-		equivocation_proof.set_id(),
-		equivocation_proof.round(),
-	);
-
-	let is_known_offence = T::HandleEquivocation::is_known_offence(&[offender], &time_slot);
-
-	if is_known_offence {
-		Err(InvalidTransaction::Stale.into())
-	} else {
-		Ok(())
-	}
-}
-
-/// A grandpa equivocation offence report.
-#[allow(dead_code)]
-pub struct GrandpaEquivocationOffence<FullIdentification> {
-	/// Time slot at which this incident happened.
-	pub time_slot: GrandpaTimeSlot,
-	/// The session index in which the incident happened.
-	pub session_index: SessionIndex,
-	/// The size of the validator set at the time of the offence.
-	pub validator_set_count: u32,
-	/// The authority which produced this equivocation.
-	pub offender: FullIdentification,
-}
-
-/// An interface for types that will be used as GRANDPA offences and must also
-/// implement the `Offence` trait. This trait provides a constructor that is
-/// provided all available data during processing of GRANDPA equivocations.
-pub trait GrandpaOffence<FullIdentification>: Offence<FullIdentification> {
-	/// Create a new GRANDPA offence using the given equivocation details.
-	fn new(
-		session_index: SessionIndex,
-		validator_set_count: u32,
-		offender: FullIdentification,
-		set_id: SetId,
-		round: RoundNumber,
-	) -> Self;
-
-	/// Create a new GRANDPA offence time slot.
-	fn new_time_slot(set_id: SetId, round: RoundNumber) -> Self::TimeSlot;
-}
-
-impl<FullIdentification: Clone> GrandpaOffence<FullIdentification>
-	for GrandpaEquivocationOffence<FullIdentification>
-{
-	fn new(
-		session_index: SessionIndex,
-		validator_set_count: u32,
-		offender: FullIdentification,
-		set_id: SetId,
-		round: RoundNumber,
-	) -> Self {
-		GrandpaEquivocationOffence {
-			session_index,
-			validator_set_count,
-			offender,
-			time_slot: GrandpaTimeSlot { set_id, round },
-		}
-	}
-
-	fn new_time_slot(set_id: SetId, round: RoundNumber) -> Self::TimeSlot {
-		GrandpaTimeSlot { set_id, round }
-	}
-}
-
-impl<FullIdentification: Clone> Offence<FullIdentification>
-	for GrandpaEquivocationOffence<FullIdentification>
-{
-	const ID: Kind = *b"grandpa:equivoca";
-	type TimeSlot = GrandpaTimeSlot;
-
-	fn offenders(&self) -> Vec<FullIdentification> {
-		vec![self.offender.clone()]
-	}
-
-	fn session_index(&self) -> SessionIndex {
-		self.session_index
-	}
-
-	fn validator_set_count(&self) -> u32 {
-		self.validator_set_count
-	}
-
-	fn time_slot(&self) -> Self::TimeSlot {
-		self.time_slot
-	}
-
-	fn slash_fraction(&self, offenders_count: u32) -> Perbill {
-		// the formula is min((3k / n)^2, 1)
-		let x = Perbill::from_rational(3 * offenders_count, self.validator_set_count);
-		// _ ^ 2
-		x.square()
-	}
-}

substrate/frame/grandpa/src/lib.rs

@@ -28,29 +28,29 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
-// re-export since this is necessary for `impl_apis` in runtime.
-pub use sp_consensus_grandpa as fg_primitives;
-
-use sp_std::prelude::*;
+// Re-export since this is necessary for `impl_apis` in runtime.
+pub use sp_consensus_grandpa::{
+	self as fg_primitives, AuthorityId, AuthorityList, AuthorityWeight, VersionedAuthorityList,
+};
 
 use codec::{self as codec, Decode, Encode, MaxEncodedLen};
-pub use fg_primitives::{AuthorityId, AuthorityList, AuthorityWeight, VersionedAuthorityList};
-use fg_primitives::{
-	ConsensusLog, EquivocationProof, ScheduledChange, SetId, GRANDPA_AUTHORITIES_KEY,
-	GRANDPA_ENGINE_ID, RUNTIME_LOG_TARGET as LOG_TARGET,
-};
 use frame_support::{
 	dispatch::{DispatchResultWithPostInfo, Pays},
 	pallet_prelude::Get,
 	storage,
-	traits::{KeyOwnerProofSystem, OneSessionHandler},
+	traits::OneSessionHandler,
 	weights::Weight,
 	WeakBoundedVec,
 };
 use scale_info::TypeInfo;
-use sp_runtime::{generic::DigestItem, traits::Zero, DispatchResult, KeyTypeId};
+use sp_consensus_grandpa::{
+	ConsensusLog, EquivocationProof, ScheduledChange, SetId, GRANDPA_AUTHORITIES_KEY,
+	GRANDPA_ENGINE_ID, RUNTIME_LOG_TARGET as LOG_TARGET,
+};
+use sp_runtime::{generic::DigestItem, traits::Zero, DispatchResult};
 use sp_session::{GetSessionNumber, GetValidatorCount};
-use sp_staking::SessionIndex;
+use sp_staking::{offence::OffenceReportSystem, SessionIndex};
+use sp_std::prelude::*;
 
 mod default_weights;
 mod equivocation;
@@ -63,10 +63,7 @@ mod mock;
 #[cfg(all(feature = "std", test))]
 mod tests;
 
-pub use equivocation::{
-	EquivocationHandler, GrandpaEquivocationOffence, GrandpaOffence, GrandpaTimeSlot,
-	HandleEquivocation,
-};
+pub use equivocation::{EquivocationOffence, EquivocationReportSystem, GrandpaTimeSlot};
 
 pub use pallet::*;
 
@@ -91,30 +88,6 @@ pub mod pallet {
 			+ Into<<Self as frame_system::Config>::RuntimeEvent>
 			+ IsType<<Self as frame_system::Config>::RuntimeEvent>;
 
-		/// The proof of key ownership, used for validating equivocation reports
-		/// The proof must include the session index and validator count of the
-		/// session at which the equivocation occurred.
-		type KeyOwnerProof: Parameter + GetSessionNumber + GetValidatorCount;
-
-		/// The identification of a key owner, used when reporting equivocations.
-		type KeyOwnerIdentification: Parameter;
-
-		/// A system for proving ownership of keys, i.e. that a given key was part
-		/// of a validator set, needed for validating equivocation reports.
-		type KeyOwnerProofSystem: KeyOwnerProofSystem<
-			(KeyTypeId, AuthorityId),
-			Proof = Self::KeyOwnerProof,
-			IdentificationTuple = Self::KeyOwnerIdentification,
-		>;
-
-		/// The equivocation handling subsystem, defines methods to report an
-		/// offence (after the equivocation has been validated) and for submitting a
-		/// transaction to report an equivocation (from an offchain context).
-		/// NOTE: when enabling equivocation handling (i.e. this type isn't set to
-		/// `()`) you must use this pallet's `ValidateUnsigned` in the runtime
-		/// definition.
-		type HandleEquivocation: HandleEquivocation<Self>;
-
 		/// Weights for this pallet.
 		type WeightInfo: WeightInfo;
 
@@ -130,6 +103,19 @@ pub mod pallet {
 		/// can be zero.
 		#[pallet::constant]
 		type MaxSetIdSessionEntries: Get<u64>;
+
+		/// The proof of key ownership, used for validating equivocation reports
+		/// The proof include the session index and validator count of the
+		/// session at which the equivocation occurred.
+		type KeyOwnerProof: Parameter + GetSessionNumber + GetValidatorCount;
+
+		/// The equivocation handling subsystem, defines methods to check/report an
+		/// offence and for submitting a transaction to report an equivocation
+		/// (from an offchain context).
+		type EquivocationReportSystem: OffenceReportSystem<
+			Option<Self::AccountId>,
+			(EquivocationProof<Self::Hash, Self::BlockNumber>, Self::KeyOwnerProof),
+		>;
 	}
 
 	#[pallet::hooks]
@@ -211,7 +197,12 @@ pub mod pallet {
 		) -> DispatchResultWithPostInfo {
 			let reporter = ensure_signed(origin)?;
 
-			Self::do_report_equivocation(Some(reporter), *equivocation_proof, key_owner_proof)
+			T::EquivocationReportSystem::process_evidence(
+				Some(reporter),
+				(*equivocation_proof, key_owner_proof),
+			)?;
+			// Waive the fee since the report is valid and beneficial
+			Ok(Pays::No.into())
 		}
 
 		/// Report voter equivocation/misbehavior. This method will verify the
@@ -232,11 +223,11 @@ pub mod pallet {
 		) -> DispatchResultWithPostInfo {
 			ensure_none(origin)?;
 
-			Self::do_report_equivocation(
-				T::HandleEquivocation::block_author(),
-				*equivocation_proof,
-				key_owner_proof,
-			)
+			T::EquivocationReportSystem::process_evidence(
+				None,
+				(*equivocation_proof, key_owner_proof),
+			)?;
+			Ok(Pays::No.into())
 		}
 
 		/// Note that the current authority set of the GRANDPA finality gadget has stalled.
@@ -352,7 +343,7 @@ pub mod pallet {
 	#[pallet::genesis_build]
 	impl<T: Config> GenesisBuild<T> for GenesisConfig {
 		fn build(&self) {
-			CurrentSetId::<T>::put(fg_primitives::SetId::default());
+			CurrentSetId::<T>::put(SetId::default());
 			Pallet::<T>::initialize(&self.authorities)
 		}
 	}
@@ -532,74 +523,6 @@ impl<T: Config> Pallet<T> {
 		SetIdSession::<T>::insert(0, 0);
 	}
 
-	fn do_report_equivocation(
-		reporter: Option<T::AccountId>,
-		equivocation_proof: EquivocationProof<T::Hash, T::BlockNumber>,
-		key_owner_proof: T::KeyOwnerProof,
-	) -> DispatchResultWithPostInfo {
-		// we check the equivocation within the context of its set id (and
-		// associated session) and round. we also need to know the validator
-		// set count when the offence since it is required to calculate the
-		// slash amount.
-		let set_id = equivocation_proof.set_id();
-		let round = equivocation_proof.round();
-		let session_index = key_owner_proof.session();
-		let validator_count = key_owner_proof.validator_count();
-
-		// validate the key ownership proof extracting the id of the offender.
-		let offender = T::KeyOwnerProofSystem::check_proof(
-			(fg_primitives::KEY_TYPE, equivocation_proof.offender().clone()),
-			key_owner_proof,
-		)
-		.ok_or(Error::<T>::InvalidKeyOwnershipProof)?;
-
-		// validate equivocation proof (check votes are different and
-		// signatures are valid).
-		if !sp_consensus_grandpa::check_equivocation_proof(equivocation_proof) {
-			return Err(Error::<T>::InvalidEquivocationProof.into())
-		}
-
-		// fetch the current and previous sets last session index. on the
-		// genesis set there's no previous set.
-		let previous_set_id_session_index = if set_id == 0 {
-			None
-		} else {
-			let session_index =
-				Self::session_for_set(set_id - 1).ok_or(Error::<T>::InvalidEquivocationProof)?;
-
-			Some(session_index)
-		};
-
-		let set_id_session_index =
-			Self::session_for_set(set_id).ok_or(Error::<T>::InvalidEquivocationProof)?;
-
-		// check that the session id for the membership proof is within the
-		// bounds of the set id reported in the equivocation.
-		if session_index > set_id_session_index ||
-			previous_set_id_session_index
-				.map(|previous_index| session_index <= previous_index)
-				.unwrap_or(false)
-		{
-			return Err(Error::<T>::InvalidEquivocationProof.into())
-		}
-
-		// report to the offences module rewarding the sender.
-		T::HandleEquivocation::report_offence(
-			reporter.into_iter().collect(),
-			<T::HandleEquivocation as HandleEquivocation<T>>::Offence::new(
-				session_index,
-				validator_count,
-				offender,
-				set_id,
-				round,
-			),
-		)
-		.map_err(|_| Error::<T>::DuplicateOffenceReport)?;
-
-		// waive the fee since the report is valid and beneficial
-		Ok(Pays::No.into())
-	}
-
 	/// Submits an extrinsic to report an equivocation. This method will create
 	/// an unsigned extrinsic with a call to `report_equivocation_unsigned` and
 	/// will push the transaction to the pool. Only useful in an offchain
@@ -608,11 +531,7 @@ impl<T: Config> Pallet<T> {
 		equivocation_proof: EquivocationProof<T::Hash, T::BlockNumber>,
 		key_owner_proof: T::KeyOwnerProof,
 	) -> Option<()> {
-		T::HandleEquivocation::submit_unsigned_equivocation_report(
-			equivocation_proof,
-			key_owner_proof,
-		)
-		.ok()
+		T::EquivocationReportSystem::publish_evidence((equivocation_proof, key_owner_proof)).ok()
 	}
 
 	fn on_stalled(further_wait: T::BlockNumber, median: T::BlockNumber) {

substrate/frame/grandpa/src/mock.rs

@@ -225,22 +225,14 @@ parameter_types! {
 impl Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 
-	type KeyOwnerProofSystem = Historical;
-
-	type KeyOwnerProof =
-		<Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(KeyTypeId, AuthorityId)>>::Proof;
-
-	type KeyOwnerIdentification = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(
-		KeyTypeId,
-		AuthorityId,
-	)>>::IdentificationTuple;
-
-	type HandleEquivocation =
-		super::EquivocationHandler<Self::KeyOwnerIdentification, Offences, ReportLongevity>;
-
 	type WeightInfo = ();
 	type MaxAuthorities = ConstU32<100>;
 	type MaxSetIdSessionEntries = MaxSetIdSessionEntries;
+
+	type KeyOwnerProof = <Historical as KeyOwnerProofSystem<(KeyTypeId, AuthorityId)>>::Proof;
+
+	type EquivocationReportSystem =
+		super::EquivocationReportSystem<Self, Offences, Historical, ReportLongevity>;
 }
 
 pub fn grandpa_log(log: ConsensusLog<u64>) -> DigestItem {

substrate/frame/grandpa/src/tests.rs

@@ -21,12 +21,11 @@
 
 use super::{Call, Event, *};
 use crate::mock::*;
-use codec::Encode;
 use fg_primitives::ScheduledChange;
 use frame_support::{
 	assert_err, assert_noop, assert_ok,
 	dispatch::{GetDispatchInfo, Pays},
-	traits::{Currency, OnFinalize, OneSessionHandler},
+	traits::{Currency, KeyOwnerProofSystem, OnFinalize, OneSessionHandler},
 };
 use frame_system::{EventRecord, Phase};
 use sp_core::H256;

substrate/frame/offences/benchmarking/src/lib.rs

@@ -36,9 +36,9 @@ use sp_runtime::{
 };
 use sp_staking::offence::{Offence, ReportOffence};
 
-use pallet_babe::BabeEquivocationOffence;
+use pallet_babe::EquivocationOffence as BabeEquivocationOffence;
 use pallet_balances::Config as BalancesConfig;
-use pallet_grandpa::{GrandpaEquivocationOffence, GrandpaTimeSlot};
+use pallet_grandpa::{EquivocationOffence as GrandpaEquivocationOffence, GrandpaTimeSlot};
 use pallet_im_online::{Config as ImOnlineConfig, Pallet as ImOnline, UnresponsivenessOffence};
 use pallet_offences::{Config as OffencesConfig, Pallet as Offences};
 use pallet_session::{

substrate/frame/offences/src/lib.rs

@@ -112,10 +112,10 @@ pub mod pallet {
 	}
 }
 
-impl<T: Config, O: Offence<T::IdentificationTuple>>
-	ReportOffence<T::AccountId, T::IdentificationTuple, O> for Pallet<T>
+impl<T, O> ReportOffence<T::AccountId, T::IdentificationTuple, O> for Pallet<T>
 where
-	T::IdentificationTuple: Clone,
+	T: Config,
+	O: Offence<T::IdentificationTuple>,
 {
 	fn report_offence(reporters: Vec<T::AccountId>, offence: O) -> Result<(), OffenceError> {
 		let offenders = offence.offenders();

substrate/frame/offences/src/mock.rs

@@ -134,17 +134,17 @@ pub fn offence_reports(kind: Kind, time_slot: u128) -> Vec<OffenceDetails<u64, u
 }
 
 #[derive(Clone)]
-pub struct Offence<T> {
+pub struct Offence {
 	pub validator_set_count: u32,
-	pub offenders: Vec<T>,
+	pub offenders: Vec<u64>,
 	pub time_slot: u128,
 }
 
-impl<T: Clone> offence::Offence<T> for Offence<T> {
+impl offence::Offence<u64> for Offence {
 	const ID: offence::Kind = KIND;
 	type TimeSlot = u128;
 
-	fn offenders(&self) -> Vec<T> {
+	fn offenders(&self) -> Vec<u64> {
 		self.offenders.clone()
 	}
 
@@ -167,5 +167,5 @@ impl<T: Clone> offence::Offence<T> for Offence<T> {
 
 /// Create the report id for the given `offender` and `time_slot` combination.
 pub fn report_id(time_slot: u128, offender: u64) -> H256 {
-	Offences::report_id::<Offence<u64>>(&time_slot, &offender)
+	Offences::report_id::<Offence>(&time_slot, &offender)
 }

substrate/frame/offences/src/tests.rs

@@ -160,20 +160,18 @@ fn doesnt_deposit_event_for_dups() {
 
 #[test]
 fn reports_if_an_offence_is_dup() {
-	type TestOffence = Offence<u64>;
-
 	new_test_ext().execute_with(|| {
 		let time_slot = 42;
 		assert_eq!(offence_reports(KIND, time_slot), vec![]);
 
 		let offence =
-			|time_slot, offenders| TestOffence { validator_set_count: 5, time_slot, offenders };
+			|time_slot, offenders| Offence { validator_set_count: 5, time_slot, offenders };
 
 		let mut test_offence = offence(time_slot, vec![0]);
 
 		// the report for authority 0 at time slot 42 should not be a known
 		// offence
-		assert!(!<Offences as ReportOffence<_, _, TestOffence>>::is_known_offence(
+		assert!(!<Offences as ReportOffence<_, _, Offence>>::is_known_offence(
 			&test_offence.offenders,
 			&test_offence.time_slot
 		));
@@ -182,7 +180,7 @@ fn reports_if_an_offence_is_dup() {
 		Offences::report_offence(vec![], test_offence.clone()).unwrap();
 
 		// the same report should be a known offence now
-		assert!(<Offences as ReportOffence<_, _, TestOffence>>::is_known_offence(
+		assert!(<Offences as ReportOffence<_, _, Offence>>::is_known_offence(
 			&test_offence.offenders,
 			&test_offence.time_slot
 		));
@@ -197,7 +195,7 @@ fn reports_if_an_offence_is_dup() {
 		test_offence.offenders.push(1);
 
 		// it should not be a known offence anymore
-		assert!(!<Offences as ReportOffence<_, _, TestOffence>>::is_known_offence(
+		assert!(!<Offences as ReportOffence<_, _, Offence>>::is_known_offence(
 			&test_offence.offenders,
 			&test_offence.time_slot
 		));
@@ -208,7 +206,7 @@ fn reports_if_an_offence_is_dup() {
 		// creating a new offence for the same authorities on the next slot
 		// should be considered a new offence and thefore not known
 		let test_offence_next_slot = offence(time_slot + 1, vec![0, 1]);
-		assert!(!<Offences as ReportOffence<_, _, TestOffence>>::is_known_offence(
+		assert!(!<Offences as ReportOffence<_, _, Offence>>::is_known_offence(
 			&test_offence_next_slot.offenders,
 			&test_offence_next_slot.time_slot
 		));

substrate/primitives/consensus/grandpa/src/lib.rs

@@ -240,6 +240,9 @@ pub struct EquivocationProof<H, N> {
 	equivocation: Equivocation<H, N>,
 }
 
+// Don't bother the grandpa crate...
+impl<H: PartialEq, N: PartialEq> Eq for EquivocationProof<H, N> {}
+
 impl<H, N> EquivocationProof<H, N> {
 	/// Create a new `EquivocationProof` for the given set id and using the
 	/// given equivocation as proof.

substrate/primitives/consensus/slots/src/lib.rs

@@ -130,7 +130,7 @@ impl SlotDuration {
 /// produces more than one block on the same slot. The proof of equivocation
 /// are the given distinct headers that were signed by the validator and which
 /// include the slot number.
-#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)]
+#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo, Eq)]
 pub struct EquivocationProof<Header, Id> {
 	/// Returns the authority id of the equivocator.
 	pub offender: Id,

substrate/primitives/staking/src/offence.rs

@@ -18,10 +18,10 @@
 //! Common traits and types that are useful for describing offences for usage in environments
 //! that use staking.
 
-use sp_std::vec::Vec;
-
 use codec::{Decode, Encode};
-use sp_runtime::Perbill;
+use sp_core::Get;
+use sp_runtime::{transaction_validity::TransactionValidityError, DispatchError, Perbill};
+use sp_std::vec::Vec;
 
 use crate::SessionIndex;
 
@@ -209,3 +209,68 @@ pub struct OffenceDetails<Reporter, Offender> {
 	/// particular reporters.
 	pub reporters: Vec<Reporter>,
 }
+
+/// An abstract system to publish, check and process offence evidences.
+///
+/// Implementation details are left opaque and we don't assume any specific usage
+/// scenario for this trait at this level. The main goal is to group together some
+/// common actions required during a typical offence report flow.
+///
+/// Even though this trait doesn't assume too much, this is a general guideline
+/// for a typical usage scenario:
+///
+/// 1. An offence is detected and an evidence is submitted on-chain via the
+///    [`OffenceReportSystem::publish_evidence`] method. This will construct
+///    and submit an extrinsic transaction containing the offence evidence.
+///
+/// 2. If the extrinsic is unsigned then the transaction receiver may want to
+///    perform some preliminary checks before further processing. This is a good
+///    place to call the [`OffenceReportSystem::check_evidence`] method.
+///
+/// 3. Finally the report extrinsic is executed on-chain. This is where the user
+///    calls the [`OffenceReportSystem::process_evidence`] to consume the offence
+///    report and enact any required action.
+pub trait OffenceReportSystem<Reporter, Evidence> {
+	/// Longevity, in blocks, for the evidence report validity.
+	///
+	/// For example, when using the staking pallet this should be set equal
+	/// to the bonding duration in blocks, not eras.
+	type Longevity: Get<u64>;
+
+	/// Publish an offence evidence.
+	///
+	/// Common usage: submit the evidence on-chain via some kind of extrinsic.
+	fn publish_evidence(evidence: Evidence) -> Result<(), ()>;
+
+	/// Check an offence evidence.
+	///
+	/// Common usage: preliminary validity check before execution
+	/// (e.g. for unsigned extrinsic quick checks).
+	fn check_evidence(evidence: Evidence) -> Result<(), TransactionValidityError>;
+
+	/// Process an offence evidence.
+	///
+	/// Common usage: enact some form of slashing directly or by forwarding
+	/// the evidence to a lower level specialized subsystem (e.g. a handler
+	/// implementing `ReportOffence` trait).
+	fn process_evidence(reporter: Reporter, evidence: Evidence) -> Result<(), DispatchError>;
+}
+
+/// Dummy offence report system.
+///
+/// Doesn't do anything special and returns `Ok(())` for all the actions.
+impl<Reporter, Evidence> OffenceReportSystem<Reporter, Evidence> for () {
+	type Longevity = ();
+
+	fn publish_evidence(_evidence: Evidence) -> Result<(), ()> {
+		Ok(())
+	}
+
+	fn check_evidence(_evidence: Evidence) -> Result<(), TransactionValidityError> {
+		Ok(())
+	}
+
+	fn process_evidence(_reporter: Reporter, _evidence: Evidence) -> Result<(), DispatchError> {
+		Ok(())
+	}
+}

substrate/test-utils/runtime/src/lib.rs

@@ -28,7 +28,7 @@ use scale_info::TypeInfo;
 use sp_std::{marker::PhantomData, prelude::*};
 
 use sp_application_crypto::{ecdsa, ed25519, sr25519, RuntimeAppPublic};
-use sp_core::{offchain::KeyTypeId, OpaqueMetadata, RuntimeDebug};
+use sp_core::{OpaqueMetadata, RuntimeDebug};
 use sp_trie::{
 	trie_types::{TrieDBBuilder, TrieDBMutBuilderV1},
 	PrefixedMemoryDB, StorageProof,
@@ -39,7 +39,7 @@ use cfg_if::cfg_if;
 use frame_support::{
 	dispatch::RawOrigin,
 	parameter_types,
-	traits::{CallerTrait, ConstU32, ConstU64, CrateVersion, KeyOwnerProofSystem},
+	traits::{CallerTrait, ConstU32, ConstU64, CrateVersion},
 	weights::{RuntimeDbWeight, Weight},
 };
 use frame_system::limits::{BlockLength, BlockWeights};
@@ -659,21 +659,10 @@ impl pallet_babe::Config for Runtime {
 	// pallet_babe::SameAuthoritiesForever.
 	type EpochChangeTrigger = pallet_babe::ExternalTrigger;
 	type DisabledValidators = ();
-
-	type KeyOwnerProofSystem = ();
-
-	type KeyOwnerProof =
-		<Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(KeyTypeId, AuthorityId)>>::Proof;
-
-	type KeyOwnerIdentification = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(
-		KeyTypeId,
-		AuthorityId,
-	)>>::IdentificationTuple;
-
-	type HandleEquivocation = ();
 	type WeightInfo = ();
-
 	type MaxAuthorities = ConstU32<10>;
+	type KeyOwnerProof = sp_core::Void;
+	type EquivocationReportSystem = ();
 }
 
 /// Adds one to the given input and returns the final result.