From 66f3d9e2372f9e2039b2bca47cfecc8fc7295b83 Mon Sep 17 00:00:00 2001 From: Davide Galassi <davxy@datawok.net> Date: Sat, 11 Mar 2023 19:05:55 +0100 Subject: [PATCH] BEEFY: introduce offence report system (#13564) * Trivial adjustments to beefy and grandpa pallets * Introduce offence report system to beefy pallet * Minor adjustments * Fix beefy-mmr mock * Apply suggestions from code review Co-authored-by: Anton <anton.kalyaev@gmail.com> --------- Co-authored-by: Anton <anton.kalyaev@gmail.com> --- substrate/frame/babe/src/equivocation.rs | 14 +- substrate/frame/beefy-mmr/src/mock.rs | 14 +- substrate/frame/beefy/src/equivocation.rs | 412 +++++++----------- substrate/frame/beefy/src/lib.rs | 130 ++---- substrate/frame/beefy/src/mock.rs | 12 +- substrate/frame/grandpa/src/equivocation.rs | 33 +- substrate/frame/grandpa/src/lib.rs | 3 +- substrate/frame/grandpa/src/mock.rs | 3 - substrate/frame/grandpa/src/tests.rs | 12 +- .../frame/offences/benchmarking/src/lib.rs | 4 +- 10 files changed, 233 insertions(+), 404 deletions(-) diff --git a/substrate/frame/babe/src/equivocation.rs b/substrate/frame/babe/src/equivocation.rs index ed98385a744..3a14cacc905 100644 --- a/substrate/frame/babe/src/equivocation.rs +++ b/substrate/frame/babe/src/equivocation.rs @@ -95,14 +95,16 @@ impl<Offender: Clone> Offence<Offender> for EquivocationOffence<Offender> { } } -/// Babe equivocation offence system. +/// BABE equivocation offence report system. /// -/// This type implements `OffenceReportSystem` +/// This type implements `OffenceReportSystem` such that: +/// - Equivocation reports are published on-chain as unsigned extrinsic via +/// `offchain::SendTransactioinsTypes`. +/// - On-chain validity checks and processing are mostly delegated to the user provided generic +/// types implementing `KeyOwnerProofSystem` and `ReportOffence` traits. +/// - Offence reporter for unsigned transactions is fetched via the the authorship pallet. 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::Header>, T::KeyOwnerProof)> for EquivocationReportSystem<T, R, P, L> @@ -131,7 +133,7 @@ where }; let res = SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into()); match res { - Ok(()) => info!(target: LOG_TARGET, "Submitted equivocation report."), + Ok(_) => info!(target: LOG_TARGET, "Submitted equivocation report"), Err(e) => error!(target: LOG_TARGET, "Error submitting equivocation report: {:?}", e), } res diff --git a/substrate/frame/beefy-mmr/src/mock.rs b/substrate/frame/beefy-mmr/src/mock.rs index d31effc9ab5..8b3bedcb960 100644 --- a/substrate/frame/beefy-mmr/src/mock.rs +++ b/substrate/frame/beefy-mmr/src/mock.rs @@ -21,11 +21,11 @@ use codec::Encode; use frame_support::{ construct_runtime, parameter_types, sp_io::TestExternalities, - traits::{ConstU16, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem}, + traits::{ConstU16, ConstU32, ConstU64, GenesisBuild}, BasicExternalities, }; use sp_consensus_beefy::mmr::MmrLeafVersion; -use sp_core::{crypto::KeyTypeId, Hasher, H256}; +use sp_core::{Hasher, H256}; use sp_runtime::{ app_crypto::ecdsa::Public, impl_opaque_keys, @@ -124,18 +124,12 @@ impl pallet_mmr::Config for Test { impl pallet_beefy::Config for Test { type BeefyId = BeefyId; - type KeyOwnerProofSystem = (); - type KeyOwnerProof = - <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(KeyTypeId, BeefyId)>>::Proof; - type KeyOwnerIdentification = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<( - KeyTypeId, - BeefyId, - )>>::IdentificationTuple; - type HandleEquivocation = (); type MaxAuthorities = ConstU32<100>; type MaxSetIdSessionEntries = ConstU64<100>; type OnNewValidatorSet = BeefyMmr; type WeightInfo = (); + type KeyOwnerProof = sp_core::Void; + type EquivocationReportSystem = (); } parameter_types! { diff --git a/substrate/frame/beefy/src/equivocation.rs b/substrate/frame/beefy/src/equivocation.rs index cc04f316c36..f83b037dcd2 100644 --- a/substrate/frame/beefy/src/equivocation.rs +++ b/substrate/frame/beefy/src/equivocation.rs @@ -34,179 +34,205 @@ //! that the `ValidateUnsigned` for the BEEFY pallet is used in the runtime //! definition. -use sp_std::prelude::*; - use codec::{self as codec, Decode, Encode}; use frame_support::{ log, traits::{Get, KeyOwnerProofSystem}, }; -use frame_system::pallet_prelude::BlockNumberFor; -use sp_consensus_beefy::{EquivocationProof, ValidatorSetId}; +use log::{error, info}; +use sp_consensus_beefy::{EquivocationProof, ValidatorSetId, KEY_TYPE}; use sp_runtime::{ transaction_validity::{ InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, TransactionValidityError, ValidTransaction, }, - DispatchResult, Perbill, RuntimeAppPublic, + DispatchError, KeyTypeId, Perbill, RuntimeAppPublic, }; +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 BEEFY. -/// 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: BeefyOffence<BlockNumberFor<T>, 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< - BlockNumberFor<T>, - T::BeefyId, - <T::BeefyId as RuntimeAppPublic>::Signature, - >, - key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult; - - /// Fetch the current block author id, if defined. - fn block_author() -> Option<T::AccountId>; +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)] +pub struct TimeSlot<N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode> { + // The order of these matters for `derive(Ord)`. + /// BEEFY Set ID. + pub set_id: ValidatorSetId, + /// Round number. + pub round: N, } -impl<T: Config> HandleEquivocation<T> for () { - type Offence = BeefyEquivocationOffence<BlockNumberFor<T>, T::KeyOwnerIdentification>; - type ReportLongevity = (); +/// BEEFY equivocation offence report. +pub struct EquivocationOffence<Offender, N> +where + N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, +{ + /// Time slot at which this incident happened. + pub time_slot: TimeSlot<N>, + /// 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, +} - fn report_offence( - _reporters: Vec<T::AccountId>, - _offence: BeefyEquivocationOffence<BlockNumberFor<T>, T::KeyOwnerIdentification>, - ) -> Result<(), OffenceError> { - Ok(()) - } +impl<Offender: Clone, N> Offence<Offender> for EquivocationOffence<Offender, N> +where + N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, +{ + const ID: Kind = *b"beefy:equivocati"; + type TimeSlot = TimeSlot<N>; - fn is_known_offence( - _offenders: &[T::KeyOwnerIdentification], - _time_slot: &BeefyTimeSlot<BlockNumberFor<T>>, - ) -> bool { - true + fn offenders(&self) -> Vec<Offender> { + vec![self.offender.clone()] } - fn submit_unsigned_equivocation_report( - _equivocation_proof: EquivocationProof< - BlockNumberFor<T>, - T::BeefyId, - <T::BeefyId as RuntimeAppPublic>::Signature, - >, - _key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult { - Ok(()) + fn session_index(&self) -> SessionIndex { + self.session_index } - fn block_author() -> Option<T::AccountId> { - None + fn validator_set_count(&self) -> u32 { + self.validator_set_count } -} -/// 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<N, I, R, L, O = BeefyEquivocationOffence<N, I>> { - _phantom: sp_std::marker::PhantomData<(N, I, R, L, O)>, -} + fn time_slot(&self) -> Self::TimeSlot { + self.time_slot + } -impl<N, I, R, L, O> Default for EquivocationHandler<N, I, R, L, O> { - fn default() -> Self { - Self { _phantom: Default::default() } + // 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() } } -impl<T, R, L, O> HandleEquivocation<T> - for EquivocationHandler<BlockNumberFor<T>, T::KeyOwnerIdentification, R, L, O> +/// BEEFY equivocation offence report system. +/// +/// This type implements `OffenceReportSystem` such that: +/// - Equivocation reports are published on-chain as unsigned extrinsic via +/// `offchain::SendTransactioinsTypes`. +/// - On-chain validity checks and processing are mostly delegated to the user provided generic +/// types implementing `KeyOwnerProofSystem` and `ReportOffence` traits. +/// - Offence reporter for unsigned transactions is fetched via the the authorship pallet. +pub struct EquivocationReportSystem<T, R, P, L>(sp_std::marker::PhantomData<(T, R, P, L)>); + +/// Equivocation evidence convenience alias. +pub type EquivocationEvidenceFor<T> = ( + EquivocationProof< + <T as frame_system::Config>::BlockNumber, + <T as Config>::BeefyId, + <<T as Config>::BeefyId as RuntimeAppPublic>::Signature, + >, + <T as Config>::KeyOwnerProof, +); + +impl<T, R, P, L> OffenceReportSystem<Option<T::AccountId>, EquivocationEvidenceFor<T>> + 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, O>, - // The longevity (in blocks) that the equivocation report is valid for. When using the staking - // pallet this should be the bonding duration. + R: ReportOffence< + T::AccountId, + P::IdentificationTuple, + EquivocationOffence<P::IdentificationTuple, T::BlockNumber>, + >, + P: KeyOwnerProofSystem<(KeyTypeId, T::BeefyId), Proof = T::KeyOwnerProof>, + P::IdentificationTuple: Clone, L: Get<u64>, - // The offence type that should be used when reporting. - O: BeefyOffence<BlockNumberFor<T>, T::KeyOwnerIdentification>, { - type Offence = O; - type ReportLongevity = L; + type Longevity = 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< - BlockNumberFor<T>, - T::BeefyId, - <T::BeefyId as RuntimeAppPublic>::Signature, - >, - key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult { + fn publish_evidence(evidence: EquivocationEvidenceFor<T>) -> 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 BEEFY 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: EquivocationEvidenceFor<T>, + ) -> 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_id().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 = TimeSlot { + set_id: equivocation_proof.set_id(), + round: *equivocation_proof.round_number(), + }; + + if R::is_known_offence(&[offender], &time_slot) { + Err(InvalidTransaction::Stale.into()) + } else { + Ok(()) + } } -} -/// A round number and set id which point on the time of an offence. -#[derive(Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Encode, Decode)] -pub struct BeefyTimeSlot<N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode> { - // The order of these matters for `derive(Ord)`. - /// BEEFY Set ID. - pub set_id: ValidatorSetId, - /// Round number. - pub round: N, + fn process_evidence( + reporter: Option<T::AccountId>, + evidence: EquivocationEvidenceFor<T>, + ) -> Result<(), DispatchError> { + let (equivocation_proof, key_owner_proof) = evidence; + let reporter = reporter.or_else(|| <pallet_authorship::Pallet<T>>::author()); + let offender = equivocation_proof.offender_id().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 at the time of the offence since it is required to calculate + // the slash amount. + let set_id = equivocation_proof.set_id(); + let round = *equivocation_proof.round_number(); + let session_index = key_owner_proof.session(); + let validator_set_count = key_owner_proof.validator_count(); + + // 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)?; + + // Validate equivocation proof (check votes are different and signatures are valid). + if !sp_consensus_beefy::check_equivocation_proof(&equivocation_proof) { + return Err(Error::<T>::InvalidEquivocationProof.into()) + } + + // Check that the session id for the membership proof is within the + // bounds of the set id reported in the equivocation. + let set_id_session_index = + crate::SetIdSession::<T>::get(set_id).ok_or(Error::<T>::InvalidEquivocationProof)?; + if session_index != set_id_session_index { + return Err(Error::<T>::InvalidEquivocationProof.into()) + } + + let offence = EquivocationOffence { + time_slot: TimeSlot { set_id, round }, + session_index, + validator_set_count, + offender, + }; + + R::report_offence(reporter.into_iter().collect(), offence) + .map_err(|_| Error::<T>::DuplicateOffenceReport)?; + + Ok(()) + } } /// Methods for the `ValidateUnsigned` implementation: @@ -228,11 +254,11 @@ impl<T: Config> Pallet<T> { }, } - // check report staleness - is_known_offence::<T>(equivocation_proof, key_owner_proof)?; + 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("BeefyEquivocation") // We assign the maximum priority for any equivocation report. @@ -254,132 +280,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< - BlockNumberFor<T>, - T::BeefyId, - <T::BeefyId as RuntimeAppPublic>::Signature, - >, - key_owner_proof: &T::KeyOwnerProof, -) -> Result<(), TransactionValidityError> { - // check the membership proof to extract the offender's id, - // equivocation validity will be fully checked during the call. - let key = (sp_consensus_beefy::KEY_TYPE, equivocation_proof.offender_id().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_number(), - ); - - let is_known_offence = T::HandleEquivocation::is_known_offence(&[offender], &time_slot); - - if is_known_offence { - Err(InvalidTransaction::Stale.into()) - } else { - Ok(()) - } -} - -/// A BEEFY equivocation offence report. -pub struct BeefyEquivocationOffence<N, FullIdentification> -where - N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, -{ - /// Time slot at which this incident happened. - pub time_slot: BeefyTimeSlot<N>, - /// 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 BEEFY offences and must also -/// implement the `Offence` trait. This trait provides a constructor that is -/// provided all available data during processing of BEEFY equivocations. -pub trait BeefyOffence<N, FullIdentification>: Offence<FullIdentification> -where - N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, -{ - /// Create a new BEEFY offence using the given equivocation details. - fn new( - session_index: SessionIndex, - validator_set_count: u32, - offender: FullIdentification, - set_id: ValidatorSetId, - round: N, - ) -> Self; - - /// Create a new BEEFY offence time slot. - fn new_time_slot(set_id: ValidatorSetId, round: N) -> Self::TimeSlot; -} - -impl<N, FullIdentification: Clone> BeefyOffence<N, FullIdentification> - for BeefyEquivocationOffence<N, FullIdentification> -where - N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, -{ - fn new( - session_index: SessionIndex, - validator_set_count: u32, - offender: FullIdentification, - set_id: ValidatorSetId, - round: N, - ) -> Self { - BeefyEquivocationOffence { - session_index, - validator_set_count, - offender, - time_slot: BeefyTimeSlot { set_id, round }, - } - } - - fn new_time_slot(set_id: ValidatorSetId, round: N) -> Self::TimeSlot { - BeefyTimeSlot { set_id, round } - } -} - -impl<N, FullIdentification: Clone> Offence<FullIdentification> - for BeefyEquivocationOffence<N, FullIdentification> -where - N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, -{ - const ID: Kind = *b"beefy:equivocati"; - type TimeSlot = BeefyTimeSlot<N>; - - 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() - } -} diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index 698e6e73312..945b32c12fe 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -23,7 +23,7 @@ use frame_support::{ dispatch::{DispatchResultWithPostInfo, Pays}, log, pallet_prelude::*, - traits::{Get, KeyOwnerProofSystem, OneSessionHandler}, + traits::{Get, OneSessionHandler}, weights::Weight, BoundedSlice, BoundedVec, Parameter, }; @@ -34,10 +34,10 @@ use frame_system::{ use sp_runtime::{ generic::DigestItem, traits::{IsMember, Member}, - KeyTypeId, RuntimeAppPublic, + RuntimeAppPublic, }; use sp_session::{GetSessionNumber, GetValidatorCount}; -use sp_staking::SessionIndex; +use sp_staking::{offence::OffenceReportSystem, SessionIndex}; use sp_std::prelude::*; use sp_consensus_beefy::{ @@ -52,11 +52,11 @@ mod mock; #[cfg(test)] mod tests; -pub use crate::equivocation::{ - BeefyEquivocationOffence, BeefyOffence, BeefyTimeSlot, EquivocationHandler, HandleEquivocation, -}; +pub use crate::equivocation::{EquivocationOffence, EquivocationReportSystem, TimeSlot}; pub use pallet::*; +use crate::equivocation::EquivocationEvidenceFor; + const LOG_TARGET: &str = "runtime::beefy"; #[frame_support::pallet] @@ -74,30 +74,6 @@ pub mod pallet { + MaybeSerializeDeserialize + MaxEncodedLen; - /// 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, Self::BeefyId), - Proof = Self::KeyOwnerProof, - IdentificationTuple = Self::KeyOwnerIdentification, - >; - - /// 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; - - /// 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>; - /// The maximum number of authorities that can be added. #[pallet::constant] type MaxAuthorities: Get<u32>; @@ -120,6 +96,19 @@ pub mod pallet { /// Weights for this pallet. type WeightInfo: WeightInfo; + + /// 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 publish, check and process an equivocation offence. + type EquivocationReportSystem: OffenceReportSystem< + Option<Self::AccountId>, + EquivocationEvidenceFor<Self>, + >; } #[pallet::pallet] @@ -229,7 +218,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 @@ -256,20 +250,22 @@ 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()) } } #[pallet::validate_unsigned] impl<T: Config> ValidateUnsigned for Pallet<T> { type Call = Call<T>; + fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> { Self::pre_dispatch(call) } + fn validate_unsigned(source: TransactionSource, call: &Self::Call) -> TransactionValidity { Self::validate_unsigned(source, call) } @@ -295,11 +291,7 @@ impl<T: Config> Pallet<T> { >, 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 change_authorities( @@ -368,62 +360,6 @@ impl<T: Config> Pallet<T> { Ok(()) } - - fn do_report_equivocation( - reporter: Option<T::AccountId>, - equivocation_proof: EquivocationProof< - BlockNumberFor<T>, - T::BeefyId, - <T::BeefyId as RuntimeAppPublic>::Signature, - >, - 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 at the time of the offence since it is required to calculate - // the slash amount. - let set_id = equivocation_proof.set_id(); - let round = *equivocation_proof.round_number(); - let offender_id = equivocation_proof.offender_id().clone(); - 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( - (sp_consensus_beefy::KEY_TYPE, offender_id), - key_owner_proof, - ) - .ok_or(Error::<T>::InvalidKeyOwnershipProof)?; - - // validate equivocation proof (check votes are different and signatures are valid). - if !sp_consensus_beefy::check_equivocation_proof(&equivocation_proof) { - return Err(Error::<T>::InvalidEquivocationProof.into()) - } - - // check that the session id for the membership proof is within the - // bounds of the set id reported in the equivocation. - let set_id_session_index = - Self::session_for_set(set_id).ok_or(Error::<T>::InvalidEquivocationProof)?; - if session_index != set_id_session_index { - 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()) - } } impl<T: Config> sp_runtime::BoundToRuntimeAppPublic for Pallet<T> { diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index 72e3f83dff9..c9296623511 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -116,19 +116,13 @@ parameter_types! { impl pallet_beefy::Config for Test { type BeefyId = BeefyId; - type KeyOwnerProofSystem = Historical; - type KeyOwnerProof = - <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(KeyTypeId, BeefyId)>>::Proof; - type KeyOwnerIdentification = <Self::KeyOwnerProofSystem as KeyOwnerProofSystem<( - KeyTypeId, - BeefyId, - )>>::IdentificationTuple; - type HandleEquivocation = - super::EquivocationHandler<u64, Self::KeyOwnerIdentification, Offences, ReportLongevity>; type MaxAuthorities = ConstU32<100>; type MaxSetIdSessionEntries = MaxSetIdSessionEntries; type OnNewValidatorSet = (); type WeightInfo = (); + type KeyOwnerProof = <Historical as KeyOwnerProofSystem<(KeyTypeId, BeefyId)>>::Proof; + type EquivocationReportSystem = + super::EquivocationReportSystem<Self, Offences, Historical, ReportLongevity>; } parameter_types! { diff --git a/substrate/frame/grandpa/src/equivocation.rs b/substrate/frame/grandpa/src/equivocation.rs index 6fd12bbdfcd..44d02663752 100644 --- a/substrate/frame/grandpa/src/equivocation.rs +++ b/substrate/frame/grandpa/src/equivocation.rs @@ -57,7 +57,7 @@ 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)] -pub struct GrandpaTimeSlot { +pub struct TimeSlot { // The order of these matters for `derive(Ord)`. /// Grandpa Set ID. pub set_id: SetId, @@ -65,10 +65,10 @@ pub struct GrandpaTimeSlot { pub round: RoundNumber, } -/// A GRANDPA equivocation offence report. +/// GRANDPA equivocation offence report. pub struct EquivocationOffence<Offender> { /// Time slot at which this incident happened. - pub time_slot: GrandpaTimeSlot, + pub time_slot: TimeSlot, /// The session index in which the incident happened. pub session_index: SessionIndex, /// The size of the validator set at the time of the offence. @@ -79,7 +79,7 @@ pub struct EquivocationOffence<Offender> { impl<Offender: Clone> Offence<Offender> for EquivocationOffence<Offender> { const ID: Kind = *b"grandpa:equivoca"; - type TimeSlot = GrandpaTimeSlot; + type TimeSlot = TimeSlot; fn offenders(&self) -> Vec<Offender> { vec![self.offender.clone()] @@ -105,15 +105,16 @@ impl<Offender: Clone> Offence<Offender> for EquivocationOffence<Offender> { } } -/// 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. +/// GRANDPA equivocation offence report system. +/// +/// This type implements `OffenceReportSystem` such that: +/// - Equivocation reports are published on-chain as unsigned extrinsic via +/// `offchain::SendTransactioinsTypes`. +/// - On-chain validity checks and processing are mostly delegated to the user provided generic +/// types implementing `KeyOwnerProofSystem` and `ReportOffence` traits. +/// - Offence reporter for unsigned transactions is fetched via the the authorship pallet. 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>, @@ -144,7 +145,7 @@ where }; let res = SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into()); match res { - Ok(()) => info!(target: LOG_TARGET, "Submitted equivocation report."), + Ok(_) => info!(target: LOG_TARGET, "Submitted equivocation report"), Err(e) => error!(target: LOG_TARGET, "Error submitting equivocation report: {:?}", e), } res @@ -160,10 +161,8 @@ where 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(), - }; + let time_slot = + TimeSlot { set_id: equivocation_proof.set_id(), round: equivocation_proof.round() }; if R::is_known_offence(&[offender], &time_slot) { Err(InvalidTransaction::Stale.into()) } else { @@ -221,7 +220,7 @@ where } let offence = EquivocationOffence { - time_slot: GrandpaTimeSlot { set_id, round }, + time_slot: TimeSlot { set_id, round }, session_index, offender, validator_set_count, diff --git a/substrate/frame/grandpa/src/lib.rs b/substrate/frame/grandpa/src/lib.rs index 2106860ea14..538cd365b5a 100644 --- a/substrate/frame/grandpa/src/lib.rs +++ b/substrate/frame/grandpa/src/lib.rs @@ -63,7 +63,7 @@ mod mock; #[cfg(all(feature = "std", test))] mod tests; -pub use equivocation::{EquivocationOffence, EquivocationReportSystem, GrandpaTimeSlot}; +pub use equivocation::{EquivocationOffence, EquivocationReportSystem, TimeSlot}; pub use pallet::*; @@ -351,6 +351,7 @@ pub mod pallet { #[pallet::validate_unsigned] impl<T: Config> ValidateUnsigned for Pallet<T> { type Call = Call<T>; + fn validate_unsigned(source: TransactionSource, call: &Self::Call) -> TransactionValidity { Self::validate_unsigned(source, call) } diff --git a/substrate/frame/grandpa/src/mock.rs b/substrate/frame/grandpa/src/mock.rs index 22ca9c624c6..4e4f2f79d04 100644 --- a/substrate/frame/grandpa/src/mock.rs +++ b/substrate/frame/grandpa/src/mock.rs @@ -224,13 +224,10 @@ parameter_types! { impl Config for Test { type RuntimeEvent = RuntimeEvent; - 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>; } diff --git a/substrate/frame/grandpa/src/tests.rs b/substrate/frame/grandpa/src/tests.rs index a956ca4bca6..16d89307bb7 100644 --- a/substrate/frame/grandpa/src/tests.rs +++ b/substrate/frame/grandpa/src/tests.rs @@ -296,12 +296,12 @@ fn schedule_resume_only_when_paused() { #[test] fn time_slot_have_sane_ord() { // Ensure that `Ord` implementation is sane. - const FIXTURE: &[GrandpaTimeSlot] = &[ - GrandpaTimeSlot { set_id: 0, round: 0 }, - GrandpaTimeSlot { set_id: 0, round: 1 }, - GrandpaTimeSlot { set_id: 1, round: 0 }, - GrandpaTimeSlot { set_id: 1, round: 1 }, - GrandpaTimeSlot { set_id: 1, round: 2 }, + const FIXTURE: &[TimeSlot] = &[ + TimeSlot { set_id: 0, round: 0 }, + TimeSlot { set_id: 0, round: 1 }, + TimeSlot { set_id: 1, round: 0 }, + TimeSlot { set_id: 1, round: 1 }, + TimeSlot { set_id: 1, round: 2 }, ]; assert!(FIXTURE.windows(2).all(|f| f[0] < f[1])); } diff --git a/substrate/frame/offences/benchmarking/src/lib.rs b/substrate/frame/offences/benchmarking/src/lib.rs index 3db937bb378..0efbdcd48ec 100644 --- a/substrate/frame/offences/benchmarking/src/lib.rs +++ b/substrate/frame/offences/benchmarking/src/lib.rs @@ -38,7 +38,9 @@ use sp_staking::offence::{Offence, ReportOffence}; use pallet_babe::EquivocationOffence as BabeEquivocationOffence; use pallet_balances::Config as BalancesConfig; -use pallet_grandpa::{EquivocationOffence as GrandpaEquivocationOffence, GrandpaTimeSlot}; +use pallet_grandpa::{ + EquivocationOffence as GrandpaEquivocationOffence, TimeSlot as GrandpaTimeSlot, +}; use pallet_im_online::{Config as ImOnlineConfig, Pallet as ImOnline, UnresponsivenessOffence}; use pallet_offences::{Config as OffencesConfig, Pallet as Offences}; use pallet_session::{ -- GitLab