diff --git a/bin/node/primitives/src/lib.rs b/bin/node/primitives/src/lib.rs index c6180b31f7e05..6e02e68add557 100644 --- a/bin/node/primitives/src/lib.rs +++ b/bin/node/primitives/src/lib.rs @@ -69,11 +69,11 @@ pub type BlockId = generic::BlockId; pub mod report { use super::{Signature, Verify}; use frame_system::offchain::AppCrypto; - use sp_core::crypto::KeyTypeId; + use sp_core::crypto::{key_types, KeyTypeId}; /// Key type for the reporting module. Used for reporting BABE and GRANDPA /// equivocations. - pub const KEY_TYPE: KeyTypeId = KeyTypeId(*b"fish"); + pub const KEY_TYPE: KeyTypeId = key_types::REPORTING; mod app { use sp_application_crypto::{app_crypto, sr25519}; diff --git a/frame/grandpa/src/equivocation.rs b/frame/grandpa/src/equivocation.rs index f9ae43021673b..2a8c3e572217f 100644 --- a/frame/grandpa/src/equivocation.rs +++ b/frame/grandpa/src/equivocation.rs @@ -29,6 +29,12 @@ //! And in a runtime context, so that the GRANDPA module can validate the //! equivocation proofs in the extrinsic and report the offences. //! +//! IMPORTANT: +//! When using this module for enabling equivocation reporting it is required +//! that the `ValidateEquivocationReport` signed extension is used in the runtime +//! definition. Failure to do so will allow invalid equivocation reports to be +//! accepted by the runtime. +//! use sp_std::prelude::*; @@ -395,12 +401,12 @@ impl GetValidatorCount for frame_support::Void { impl GetSessionNumber for sp_session::MembershipProof { fn session(&self) -> SessionIndex { - self.session() + self.session } } impl GetValidatorCount for sp_session::MembershipProof { fn validator_count(&self) -> sp_session::ValidatorCount { - self.validator_count() + self.validator_count } } diff --git a/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index 16aebe335f9ae..055607816d3f1 100644 --- a/frame/grandpa/src/lib.rs +++ b/frame/grandpa/src/lib.rs @@ -184,6 +184,10 @@ decl_error! { ChangePending, /// Cannot signal forced change so soon after last. TooSoon, + /// A key ownership proof provided as part of an equivocation report is invalid. + InvalidKeyOwnershipProof, + /// A given equivocation report is valid but already previously reported. + DuplicateOffenceReport, } } @@ -228,8 +232,9 @@ decl_module! { /// against the extracted offender. If both are valid, the offence /// will be reported. /// - /// Since the weight is 0 in order to avoid DoS pre-validation is implemented in a - /// `SignedExtension`. + /// Since the weight of the extrinsic is 0, in order to avoid DoS by + /// submission of invalid equivocation reports, a mandatory pre-validation of + /// the extrinsic is implemented in a `SignedExtension`. #[weight = 0] fn report_equivocation( origin, @@ -249,7 +254,7 @@ decl_module! { T::KeyOwnerProofSystem::check_proof( (fg_primitives::KEY_TYPE, equivocation_proof.offender().clone()), key_owner_proof, - ).ok_or("Invalid key ownership proof.")?; + ).ok_or(Error::::InvalidKeyOwnershipProof)?; // the set id and round when the offence happened let set_id = equivocation_proof.set_id(); @@ -265,7 +270,7 @@ decl_module! { set_id, round, ), - ).map_err(|_| "Duplicate offence report.")?; + ).map_err(|_| Error::::DuplicateOffenceReport)?; } fn on_finalize(block_number: T::BlockNumber) { @@ -440,9 +445,9 @@ impl Module { Self::set_grandpa_authorities(authorities); } - // NOTE: initialize first session of first set. this is necessary - // because we only update this `on_new_session` which isn't called - // for the genesis session. + // NOTE: initialize first session of first set. this is necessary for + // the genesis set and session since we only update the set -> session + // mapping whenever a new session starts, i.e. through `on_new_session`. SetIdSession::insert(0, 0); } @@ -454,10 +459,7 @@ impl Module { equivocation_proof: EquivocationProof, key_owner_proof: T::KeyOwnerProof, ) -> Option<()> { - T::HandleEquivocation::submit_equivocation_report(equivocation_proof, key_owner_proof) - .ok()?; - - Some(()) + T::HandleEquivocation::submit_equivocation_report(equivocation_proof, key_owner_proof).ok() } } diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 44d43c3a44230..f725e94a42ada 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -1006,6 +1006,8 @@ pub mod key_types { pub const AUTHORITY_DISCOVERY: KeyTypeId = KeyTypeId(*b"audi"); /// Key type for staking, built-in. Identified as `stak`. pub const STAKING: KeyTypeId = KeyTypeId(*b"stak"); + /// Key type for equivocation reporting, built-in. Identified as `fish`. + pub const REPORTING: KeyTypeId = KeyTypeId(*b"fish"); /// A key type ID useful for tests. pub const DUMMY: KeyTypeId = KeyTypeId(*b"dumy"); } diff --git a/primitives/finality-grandpa/src/lib.rs b/primitives/finality-grandpa/src/lib.rs index a84ce57c8d1f6..7cc8ddf5cc515 100644 --- a/primitives/finality-grandpa/src/lib.rs +++ b/primitives/finality-grandpa/src/lib.rs @@ -352,21 +352,11 @@ where H: Encode, N: Encode, { - localized_payload_with_buffer(round, set_id, message, buf); - - #[cfg(not(feature = "std"))] - let verify = || { - use sp_application_crypto::RuntimeAppPublic; - id.verify(&buf, signature) - }; + use sp_application_crypto::RuntimeAppPublic; - #[cfg(feature = "std")] - let verify = || { - use sp_application_crypto::Pair; - AuthorityPair::verify(signature, &buf, &id) - }; + localized_payload_with_buffer(round, set_id, message, buf); - if verify() { + if id.verify(&buf, signature) { Ok(()) } else { #[cfg(feature = "std")] @@ -501,7 +491,10 @@ sp_api::decl_runtime_apis! { /// provide the equivocation proof and a key ownership proof (should be /// obtained using `generate_key_ownership_proof`). This method will /// sign the extrinsic with any reporting keys available in the keystore - /// and will push the transaction to the pool. + /// and will push the transaction to the pool. This method returns `None` + /// when creation of the extrinsic fails, either due to unavailability + /// of keys to sign, or because equivocation reporting is disabled for + /// the given runtime (i.e. this method is hardcoded to return `None`). /// Only useful in an offchain context. fn submit_report_equivocation_extrinsic( equivocation_proof: EquivocationProof>, diff --git a/primitives/session/src/lib.rs b/primitives/session/src/lib.rs index 720dfbfcaddfb..74b4a250e6f05 100644 --- a/primitives/session/src/lib.rs +++ b/primitives/session/src/lib.rs @@ -63,18 +63,6 @@ pub struct MembershipProof { pub validator_count: ValidatorCount, } -impl MembershipProof { - /// Returns a session this proof was generated for. - pub fn session(&self) -> SessionIndex { - self.session - } - - /// Returns the validator count of the session this proof was generated for. - pub fn validator_count(&self) -> ValidatorCount { - self.validator_count - } -} - /// Generate the initial session keys with the given seeds, at the given block and store them in /// the client's keystore. #[cfg(feature = "std")]