diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 57e4470c762ac..ef90517a2a258 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/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 = - >::Proof; - - type KeyOwnerIdentification = >::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 { diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 3d160108f62bb..c507c2d8d752f 100644 --- a/bin/node/runtime/src/lib.rs +++ b/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 = >::Proof; - - type KeyOwnerIdentification = >::IdentificationTuple; - - type HandleEquivocation = - pallet_babe::EquivocationHandler; - type WeightInfo = (); type MaxAuthorities = MaxAuthorities; + type KeyOwnerProof = + >::Proof; + type EquivocationReportSystem = + pallet_babe::EquivocationReportSystem; } parameter_types! { @@ -1328,26 +1321,12 @@ parameter_types! { impl pallet_grandpa::Config for Runtime { type RuntimeEvent = RuntimeEvent; - - type KeyOwnerProofSystem = Historical; - - type KeyOwnerProof = - >::Proof; - - type KeyOwnerIdentification = >::IdentificationTuple; - - type HandleEquivocation = pallet_grandpa::EquivocationHandler< - Self::KeyOwnerIdentification, - Offences, - ReportLongevity, - >; - type WeightInfo = (); type MaxAuthorities = MaxAuthorities; type MaxSetIdSessionEntries = MaxSetIdSessionEntries; + type KeyOwnerProof = >::Proof; + type EquivocationReportSystem = + pallet_grandpa::EquivocationReportSystem; } parameter_types! { @@ -2483,10 +2462,10 @@ mod tests { fn call_size() { let size = core::mem::size_of::(); 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, ); } diff --git a/frame/babe/src/equivocation.rs b/frame/babe/src/equivocation.rs index 2a2e2b47605d1..ed98385a74474 100644 --- a/frame/babe/src/equivocation.rs +++ b/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}; - -/// 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 { - /// 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: BabeEquivocationOffence, - ) -> 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, - key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult; - - /// Fetch the current block author id, if defined. - fn block_author() -> Option; +use crate::{Call, Config, Error, Pallet, LOG_TARGET}; + +/// BABE equivocation offence report. +/// +/// When a validator released two or more blocks at the same slot. +pub struct EquivocationOffence { + /// 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 HandleEquivocation for () { - type ReportLongevity = (); +impl Offence for EquivocationOffence { + const ID: Kind = *b"babe:equivocatio"; + type TimeSlot = Slot; - fn report_offence( - _reporters: Vec, - _offence: BabeEquivocationOffence, - ) -> Result<(), OffenceError> { - Ok(()) + fn offenders(&self) -> Vec { + 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, - _key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult { - Ok(()) + fn validator_set_count(&self) -> u32 { + self.validator_set_count } - fn block_author() -> Option { - None + fn time_slot(&self) -> Self::TimeSlot { + self.slot } -} - -/// 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<(I, R, L)>, -} -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 +/// Babe equivocation offence system. +/// +/// This type implements `OffenceReportSystem` +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 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< T::AccountId, - T::KeyOwnerIdentification, - BabeEquivocationOffence, + P::IdentificationTuple, + EquivocationOffence, >, - // 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, { - type ReportLongevity = L; + type Longevity = L; - fn report_offence( - reporters: Vec, - offence: BabeEquivocationOffence, - ) -> 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, - key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult { + fn publish_evidence( + evidence: (EquivocationProof, 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::>::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::>::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 + } - Ok(()) + fn check_evidence( + evidence: (EquivocationProof, 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 block_author() -> Option { - >::author() + fn process_evidence( + reporter: Option, + evidence: (EquivocationProof, T::KeyOwnerProof), + ) -> Result<(), DispatchError> { + let (equivocation_proof, key_owner_proof) = evidence; + let reporter = reporter.or_else(|| >::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::::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::::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::::session_index_for_epoch(epoch_index) != session_index { + return Err(Error::::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::::InvalidKeyOwnershipProof)?; + + let offence = EquivocationOffence { slot, validator_set_count, offender, session_index }; + + R::report_offence(reporter.into_iter().collect(), offence) + .map_err(|_| Error::::DuplicateOffenceReport)?; + + Ok(()) } } @@ -194,11 +214,12 @@ impl Pallet { }, } - // check report staleness - is_known_offence::(equivocation_proof, key_owner_proof)?; + // Check report validity + let evidence = (*equivocation_proof.clone(), key_owner_proof.clone()); + T::EquivocationReportSystem::check_evidence(evidence)?; let longevity = - >::ReportLongevity::get(); + >::Longevity::get(); ValidTransaction::with_tag_prefix("BabeEquivocation") // We assign the maximum priority for any equivocation report. @@ -216,72 +237,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, - 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 { - /// 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 Offence - for BabeEquivocationOffence -{ - const ID: Kind = *b"babe:equivocatio"; - type TimeSlot = Slot; - - 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.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/babe/src/lib.rs b/frame/babe/src/lib.rs index bb07037ee0a12..903f6249e3310 100644 --- a/frame/babe/src/lib.rs +++ b/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; + /// Helper for weights computations + type WeightInfo: WeightInfo; + + /// Max number of authorities allowed + #[pallet::constant] + type MaxAuthorities: Get; + /// 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 check/report an + /// offence and for submitting a transaction to report an equivocation + /// (from an offchain context). + type EquivocationReportSystem: OffenceReportSystem< + Option, + (EquivocationProof, Self::KeyOwnerProof), >; - - /// 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; - - type WeightInfo: WeightInfo; - - /// Max number of authorities allowed - #[pallet::constant] - type MaxAuthorities: Get; } #[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 Pallet { /// 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::::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 Pallet { } } - fn do_report_equivocation( - reporter: Option, - equivocation_proof: EquivocationProof, - 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::::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::::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::::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::::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::::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 Pallet { equivocation_proof: EquivocationProof, 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() } } diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index b130677897883..9b832bfffdb69 100644 --- a/frame/babe/src/mock.rs +++ b/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 = - >::Proof; - - type KeyOwnerIdentification = >::IdentificationTuple; - - type HandleEquivocation = - super::EquivocationHandler; - type WeightInfo = (); type MaxAuthorities = ConstU32<10>; + type KeyOwnerProof = >::Proof; + type EquivocationReportSystem = + super::EquivocationReportSystem; } pub fn go_to_block(n: u64, s: u64) { diff --git a/frame/babe/src/tests.rs b/frame/babe/src/tests.rs index 68426d0a8636e..8b63f41b37b74 100644 --- a/frame/babe/src/tests.rs +++ b/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; diff --git a/frame/grandpa/src/equivocation.rs b/frame/grandpa/src/equivocation.rs index 7d8eb6774097c..6fd12bbdfcdb9 100644 --- a/frame/grandpa/src/equivocation.rs +++ b/frame/grandpa/src/equivocation.rs @@ -35,86 +35,73 @@ //! 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 { - /// The offence type used for reporting offences on valid equivocation reports. - type Offence: GrandpaOffence; - - /// 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, - 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 GrandpaTimeSlot { + // The order of these matters for `derive(Ord)`. + /// Grandpa Set ID. + pub set_id: SetId, + /// Round number. + pub round: RoundNumber, } -impl HandleEquivocation for () { - type Offence = GrandpaEquivocationOffence; - type ReportLongevity = (); +/// A GRANDPA equivocation offence report. +pub struct EquivocationOffence { + /// 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, +} - fn report_offence( - _reporters: Vec, - _offence: GrandpaEquivocationOffence, - ) -> Result<(), OffenceError> { - Ok(()) +impl Offence for EquivocationOffence { + const ID: Kind = *b"grandpa:equivoca"; + type TimeSlot = GrandpaTimeSlot; + + fn offenders(&self) -> Vec { + vec![self.offender.clone()] } - fn is_known_offence( - _offenders: &[T::KeyOwnerIdentification], - _time_slot: &GrandpaTimeSlot, - ) -> bool { - true + fn session_index(&self) -> SessionIndex { + self.session_index } - fn submit_unsigned_equivocation_report( - _equivocation_proof: EquivocationProof, - _key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult { - Ok(()) + fn validator_set_count(&self) -> u32 { + self.validator_set_count + } + + fn time_slot(&self) -> Self::TimeSlot { + self.time_slot } - fn block_author() -> Option { - None + // 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() } } @@ -122,75 +109,129 @@ impl HandleEquivocation for () { /// 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<(I, R, L, O)>, -} - -impl Default for EquivocationHandler { - fn default() -> Self { - Self { _phantom: Default::default() } - } -} - -impl HandleEquivocation for EquivocationHandler +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, + (EquivocationProof, T::KeyOwnerProof), + > 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, AuthorityId), Proof = T::KeyOwnerProof>, + P::IdentificationTuple: Clone, L: Get, - // The offence type that should be used when reporting. - O: GrandpaOffence, { - type Offence = O; - type ReportLongevity = L; - - fn report_offence(reporters: Vec, offence: O) -> Result<(), OffenceError> { - R::report_offence(reporters, offence) - } + type Longevity = L; - 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, - key_owner_proof: T::KeyOwnerProof, - ) -> DispatchResult { + fn publish_evidence( + evidence: (EquivocationProof, 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::>::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,), + 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: (EquivocationProof, 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(()) + } } -} -/// 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 { - // The order of these matters for `derive(Ord)`. - /// Grandpa Set ID. - pub set_id: SetId, - /// Round number. - pub round: RoundNumber, + fn process_evidence( + reporter: Option, + evidence: (EquivocationProof, T::KeyOwnerProof), + ) -> Result<(), DispatchError> { + let (equivocation_proof, key_owner_proof) = evidence; + let reporter = reporter.or_else(|| >::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::::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::::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::::get(set_id - 1) + .ok_or(Error::::InvalidEquivocationProof)?; + Some(idx) + } else { + None + }; + + let set_id_session_index = + crate::SetIdSession::::get(set_id).ok_or(Error::::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::::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::::DuplicateOffenceReport)?; + + Ok(()) + } } /// Methods for the `ValidateUnsigned` implementation: @@ -213,11 +254,12 @@ impl Pallet { }, } - // check report staleness - is_known_offence::(equivocation_proof, key_owner_proof)?; + // Check report validity + let evidence = (*equivocation_proof.clone(), key_owner_proof.clone()); + T::EquivocationReportSystem::check_evidence(evidence)?; let longevity = - >::ReportLongevity::get(); + >::Longevity::get(); ValidTransaction::with_tag_prefix("GrandpaEquivocation") // We assign the maximum priority for any equivocation report. @@ -239,118 +281,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, - 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 = >::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 { - /// 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: Offence { - /// 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 GrandpaOffence - for GrandpaEquivocationOffence -{ - 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 Offence - for GrandpaEquivocationOffence -{ - const ID: Kind = *b"grandpa:equivoca"; - type TimeSlot = GrandpaTimeSlot; - - 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/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index 85d116ee7d7e7..2106860ea1441 100644 --- a/frame/grandpa/src/lib.rs +++ b/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<::RuntimeEvent> + IsType<::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; - /// Weights for this pallet. type WeightInfo: WeightInfo; @@ -130,6 +103,19 @@ pub mod pallet { /// can be zero. #[pallet::constant] type MaxSetIdSessionEntries: Get; + + /// 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, + (EquivocationProof, 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 GenesisBuild for GenesisConfig { fn build(&self) { - CurrentSetId::::put(fg_primitives::SetId::default()); + CurrentSetId::::put(SetId::default()); Pallet::::initialize(&self.authorities) } } @@ -532,74 +523,6 @@ impl Pallet { SetIdSession::::insert(0, 0); } - fn do_report_equivocation( - reporter: Option, - equivocation_proof: EquivocationProof, - 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::::InvalidKeyOwnershipProof)?; - - // validate equivocation proof (check votes are different and - // signatures are valid). - if !sp_consensus_grandpa::check_equivocation_proof(equivocation_proof) { - return Err(Error::::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::::InvalidEquivocationProof)?; - - Some(session_index) - }; - - let set_id_session_index = - Self::session_for_set(set_id).ok_or(Error::::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::::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()) - } - /// 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 Pallet { equivocation_proof: EquivocationProof, 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) { diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index a33fbb22d6ff8..22ca9c624c626 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -225,22 +225,14 @@ parameter_types! { impl Config for Test { type RuntimeEvent = RuntimeEvent; - type KeyOwnerProofSystem = Historical; - - type KeyOwnerProof = - >::Proof; - - type KeyOwnerIdentification = >::IdentificationTuple; - - type HandleEquivocation = - super::EquivocationHandler; - type WeightInfo = (); type MaxAuthorities = ConstU32<100>; type MaxSetIdSessionEntries = MaxSetIdSessionEntries; + + type KeyOwnerProof = >::Proof; + + type EquivocationReportSystem = + super::EquivocationReportSystem; } pub fn grandpa_log(log: ConsensusLog) -> DigestItem { diff --git a/frame/grandpa/src/tests.rs b/frame/grandpa/src/tests.rs index 0bf3c8ddf63dc..a956ca4bca6e9 100644 --- a/frame/grandpa/src/tests.rs +++ b/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; diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 16d77335e1a82..3db937bb378e5 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/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::{ diff --git a/frame/offences/src/lib.rs b/frame/offences/src/lib.rs index 6ba16161b320c..8ac1ca0bd734c 100644 --- a/frame/offences/src/lib.rs +++ b/frame/offences/src/lib.rs @@ -112,10 +112,10 @@ pub mod pallet { } } -impl> - ReportOffence for Pallet +impl ReportOffence for Pallet where - T::IdentificationTuple: Clone, + T: Config, + O: Offence, { fn report_offence(reporters: Vec, offence: O) -> Result<(), OffenceError> { let offenders = offence.offenders(); diff --git a/frame/offences/src/mock.rs b/frame/offences/src/mock.rs index d0fc7377acfd6..27e1da8c4e636 100644 --- a/frame/offences/src/mock.rs +++ b/frame/offences/src/mock.rs @@ -134,17 +134,17 @@ pub fn offence_reports(kind: Kind, time_slot: u128) -> Vec { +pub struct Offence { pub validator_set_count: u32, - pub offenders: Vec, + pub offenders: Vec, pub time_slot: u128, } -impl offence::Offence for Offence { +impl offence::Offence for Offence { const ID: offence::Kind = KIND; type TimeSlot = u128; - fn offenders(&self) -> Vec { + fn offenders(&self) -> Vec { self.offenders.clone() } @@ -167,5 +167,5 @@ impl offence::Offence for Offence { /// 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::>(&time_slot, &offender) + Offences::report_id::(&time_slot, &offender) } diff --git a/frame/offences/src/tests.rs b/frame/offences/src/tests.rs index a5c18020bcaa3..81f0f44f1b939 100644 --- a/frame/offences/src/tests.rs +++ b/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; - 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!(!>::is_known_offence( + assert!(!>::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!(>::is_known_offence( + assert!(>::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!(!>::is_known_offence( + assert!(!>::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!(!>::is_known_offence( + assert!(!>::is_known_offence( &test_offence_next_slot.offenders, &test_offence_next_slot.time_slot )); diff --git a/primitives/consensus/grandpa/src/lib.rs b/primitives/consensus/grandpa/src/lib.rs index b466dcf374ccf..26cee07b80be8 100644 --- a/primitives/consensus/grandpa/src/lib.rs +++ b/primitives/consensus/grandpa/src/lib.rs @@ -240,6 +240,9 @@ pub struct EquivocationProof { equivocation: Equivocation, } +// Don't bother the grandpa crate... +impl Eq for EquivocationProof {} + impl EquivocationProof { /// Create a new `EquivocationProof` for the given set id and using the /// given equivocation as proof. diff --git a/primitives/consensus/slots/src/lib.rs b/primitives/consensus/slots/src/lib.rs index d723785278da9..1f2fa585df7f2 100644 --- a/primitives/consensus/slots/src/lib.rs +++ b/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 { /// Returns the authority id of the equivocator. pub offender: Id, diff --git a/primitives/staking/src/offence.rs b/primitives/staking/src/offence.rs index dec9756687979..6694c9055d4ff 100644 --- a/primitives/staking/src/offence.rs +++ b/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 { /// particular reporters. pub reporters: Vec, } + +/// 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 { + /// 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; + + /// 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 OffenceReportSystem 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(()) + } +} diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 1ea8b866367dd..83b4e2977b5cd 100644 --- a/test-utils/runtime/src/lib.rs +++ b/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 = - >::Proof; - - type KeyOwnerIdentification = >::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.