From 310fe4095bff177ddcfa660ac0ab06171a25a9c2 Mon Sep 17 00:00:00 2001 From: Davide Galassi 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 --------- Co-authored-by: Anton --- frame/babe/src/equivocation.rs | 14 +- frame/beefy-mmr/src/mock.rs | 14 +- frame/beefy/src/equivocation.rs | 412 ++++++++++--------------- frame/beefy/src/lib.rs | 130 ++------ frame/beefy/src/mock.rs | 12 +- frame/grandpa/src/equivocation.rs | 33 +- frame/grandpa/src/lib.rs | 3 +- frame/grandpa/src/mock.rs | 3 - frame/grandpa/src/tests.rs | 12 +- frame/offences/benchmarking/src/lib.rs | 4 +- 10 files changed, 233 insertions(+), 404 deletions(-) diff --git a/frame/babe/src/equivocation.rs b/frame/babe/src/equivocation.rs index ed98385a74474..3a14cacc905d2 100644 --- a/frame/babe/src/equivocation.rs +++ b/frame/babe/src/equivocation.rs @@ -95,14 +95,16 @@ impl Offence for EquivocationOffence { } } -/// 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(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 OffenceReportSystem, (EquivocationProof, T::KeyOwnerProof)> for EquivocationReportSystem @@ -131,7 +133,7 @@ where }; let res = SubmitTransaction::>::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/frame/beefy-mmr/src/mock.rs b/frame/beefy-mmr/src/mock.rs index d31effc9ab577..8b3bedcb960b4 100644 --- a/frame/beefy-mmr/src/mock.rs +++ b/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 = - >::Proof; - type KeyOwnerIdentification = >::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/frame/beefy/src/equivocation.rs b/frame/beefy/src/equivocation.rs index cc04f316c3638..f83b037dcd26e 100644 --- a/frame/beefy/src/equivocation.rs +++ b/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 { - /// The offence type used for reporting offences on valid equivocation reports. - type Offence: BeefyOffence, 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; - - /// Report an offence proved by the given reporters. - fn report_offence( - reporters: Vec, - 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: &>::TimeSlot, - ) -> bool; - - /// Create and dispatch an equivocation report extrinsic. - fn submit_unsigned_equivocation_report( - equivocation_proof: EquivocationProof< - BlockNumberFor, - T::BeefyId, - ::Signature, - >, - key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult; - - /// Fetch the current block author id, if defined. - fn block_author() -> Option; +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 { + // The order of these matters for `derive(Ord)`. + /// BEEFY Set ID. + pub set_id: ValidatorSetId, + /// Round number. + pub round: N, } -impl HandleEquivocation for () { - type Offence = BeefyEquivocationOffence, T::KeyOwnerIdentification>; - type ReportLongevity = (); +/// BEEFY equivocation offence report. +pub struct EquivocationOffence +where + N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, +{ + /// Time slot at which this incident happened. + 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. + pub validator_set_count: u32, + /// The authority which produced this equivocation. + pub offender: Offender, +} - fn report_offence( - _reporters: Vec, - _offence: BeefyEquivocationOffence, T::KeyOwnerIdentification>, - ) -> Result<(), OffenceError> { - Ok(()) - } +impl Offence for EquivocationOffence +where + N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, +{ + const ID: Kind = *b"beefy:equivocati"; + type TimeSlot = TimeSlot; - fn is_known_offence( - _offenders: &[T::KeyOwnerIdentification], - _time_slot: &BeefyTimeSlot>, - ) -> bool { - true + fn offenders(&self) -> Vec { + vec![self.offender.clone()] } - fn submit_unsigned_equivocation_report( - _equivocation_proof: EquivocationProof< - BlockNumberFor, - T::BeefyId, - ::Signature, - >, - _key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult { - Ok(()) + fn session_index(&self) -> SessionIndex { + self.session_index } - fn block_author() -> Option { - 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> { - _phantom: sp_std::marker::PhantomData<(N, I, R, L, O)>, -} + fn time_slot(&self) -> Self::TimeSlot { + self.time_slot + } -impl Default for EquivocationHandler { - 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 HandleEquivocation - for EquivocationHandler, 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(sp_std::marker::PhantomData<(T, R, P, L)>); + +/// Equivocation evidence convenience alias. +pub type EquivocationEvidenceFor = ( + EquivocationProof< + ::BlockNumber, + ::BeefyId, + <::BeefyId as RuntimeAppPublic>::Signature, + >, + ::KeyOwnerProof, +); + +impl OffenceReportSystem, EquivocationEvidenceFor> + for EquivocationReportSystem 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>, - // A system for reporting offences after valid equivocation reports are - // processed. - R: ReportOffence, - // 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: KeyOwnerProofSystem<(KeyTypeId, T::BeefyId), Proof = T::KeyOwnerProof>, + P::IdentificationTuple: Clone, L: Get, - // The offence type that should be used when reporting. - O: BeefyOffence, T::KeyOwnerIdentification>, { - type Offence = O; - type ReportLongevity = L; + type Longevity = L; - fn report_offence(reporters: Vec, 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::BeefyId, - ::Signature, - >, - key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult { + fn publish_evidence(evidence: EquivocationEvidenceFor) -> 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::>::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::>::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 { - >::author() + fn check_evidence( + evidence: EquivocationEvidenceFor, + ) -> 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 { - // 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, + evidence: EquivocationEvidenceFor, + ) -> Result<(), DispatchError> { + let (equivocation_proof, key_owner_proof) = evidence; + let reporter = reporter.or_else(|| >::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::::InvalidKeyOwnershipProof)?; + + // Validate equivocation proof (check votes are different and signatures are valid). + if !sp_consensus_beefy::check_equivocation_proof(&equivocation_proof) { + return Err(Error::::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::::get(set_id).ok_or(Error::::InvalidEquivocationProof)?; + if session_index != set_id_session_index { + return Err(Error::::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::::DuplicateOffenceReport)?; + + Ok(()) + } } /// Methods for the `ValidateUnsigned` implementation: @@ -228,11 +254,11 @@ impl Pallet { }, } - // check report staleness - is_known_offence::(equivocation_proof, key_owner_proof)?; + let evidence = (*equivocation_proof.clone(), key_owner_proof.clone()); + T::EquivocationReportSystem::check_evidence(evidence)?; let longevity = - >::ReportLongevity::get(); + >::Longevity::get(); ValidTransaction::with_tag_prefix("BeefyEquivocation") // We assign the maximum priority for any equivocation report. @@ -254,132 +280,10 @@ impl Pallet { pub fn pre_dispatch(call: &Call) -> Result<(), TransactionValidityError> { if let Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof } = call { - is_known_offence::(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( - equivocation_proof: &EquivocationProof< - BlockNumberFor, - T::BeefyId, - ::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 = >::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 -where - N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, -{ - /// Time slot at which this incident happened. - pub time_slot: BeefyTimeSlot, - /// 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: Offence -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 BeefyOffence - for BeefyEquivocationOffence -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 Offence - for BeefyEquivocationOffence -where - N: Copy + Clone + PartialOrd + Ord + Eq + PartialEq + Encode + Decode, -{ - const ID: Kind = *b"beefy:equivocati"; - type TimeSlot = BeefyTimeSlot; - - fn offenders(&self) -> Vec { - 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/frame/beefy/src/lib.rs b/frame/beefy/src/lib.rs index 698e6e73312f1..945b32c12fe58 100644 --- a/frame/beefy/src/lib.rs +++ b/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; - /// The maximum number of authorities that can be added. #[pallet::constant] type MaxAuthorities: Get; @@ -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, + EquivocationEvidenceFor, + >; } #[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 ValidateUnsigned for Pallet { type Call = Call; + 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 Pallet { >, 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 Pallet { Ok(()) } - - fn do_report_equivocation( - reporter: Option, - equivocation_proof: EquivocationProof< - BlockNumberFor, - T::BeefyId, - ::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::::InvalidKeyOwnershipProof)?; - - // validate equivocation proof (check votes are different and signatures are valid). - if !sp_consensus_beefy::check_equivocation_proof(&equivocation_proof) { - return Err(Error::::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::::InvalidEquivocationProof)?; - if session_index != set_id_session_index { - return Err(Error::::InvalidEquivocationProof.into()) - } - - // report to the offences module rewarding the sender. - T::HandleEquivocation::report_offence( - reporter.into_iter().collect(), - >::Offence::new( - session_index, - validator_count, - offender, - set_id, - round, - ), - ) - .map_err(|_| Error::::DuplicateOffenceReport)?; - - // waive the fee since the report is valid and beneficial - Ok(Pays::No.into()) - } } impl sp_runtime::BoundToRuntimeAppPublic for Pallet { diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index 72e3f83dff91c..c92966235118e 100644 --- a/frame/beefy/src/mock.rs +++ b/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 = - >::Proof; - type KeyOwnerIdentification = >::IdentificationTuple; - type HandleEquivocation = - super::EquivocationHandler; type MaxAuthorities = ConstU32<100>; type MaxSetIdSessionEntries = MaxSetIdSessionEntries; type OnNewValidatorSet = (); type WeightInfo = (); + type KeyOwnerProof = >::Proof; + type EquivocationReportSystem = + super::EquivocationReportSystem; } parameter_types! { diff --git a/frame/grandpa/src/equivocation.rs b/frame/grandpa/src/equivocation.rs index 6fd12bbdfcdb9..44d0266375230 100644 --- a/frame/grandpa/src/equivocation.rs +++ b/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 { /// 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 { impl Offence for EquivocationOffence { const ID: Kind = *b"grandpa:equivoca"; - type TimeSlot = GrandpaTimeSlot; + type TimeSlot = TimeSlot; fn offenders(&self) -> Vec { vec![self.offender.clone()] @@ -105,15 +105,16 @@ impl Offence for EquivocationOffence { } } -/// 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(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 OffenceReportSystem< Option, @@ -144,7 +145,7 @@ where }; let res = SubmitTransaction::>::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/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index 2106860ea1441..538cd365b5a1a 100644 --- a/frame/grandpa/src/lib.rs +++ b/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 ValidateUnsigned for Pallet { type Call = Call; + fn validate_unsigned(source: TransactionSource, call: &Self::Call) -> TransactionValidity { Self::validate_unsigned(source, call) } diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index 22ca9c624c626..4e4f2f79d040e 100644 --- a/frame/grandpa/src/mock.rs +++ b/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 = >::Proof; - type EquivocationReportSystem = super::EquivocationReportSystem; } diff --git a/frame/grandpa/src/tests.rs b/frame/grandpa/src/tests.rs index a956ca4bca6e9..16d89307bb71f 100644 --- a/frame/grandpa/src/tests.rs +++ b/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/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 3db937bb378e5..0efbdcd48ecf3 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/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::{