diff --git a/frame/im-online/src/benchmarking.rs b/frame/im-online/src/benchmarking.rs index 287a2c6fd3a73..5ab4d16c7fe08 100644 --- a/frame/im-online/src/benchmarking.rs +++ b/frame/im-online/src/benchmarking.rs @@ -29,7 +29,7 @@ use sp_runtime::traits::{ValidateUnsigned, Zero}; use sp_runtime::transaction_validity::TransactionSource; use frame_support::traits::UnfilteredDispatchable; -use crate::Module as ImOnline; +use crate::Pallet as ImOnline; const MAX_KEYS: u32 = 1000; const MAX_EXTERNAL_ADDRESSES: u32 = 100; diff --git a/frame/im-online/src/lib.rs b/frame/im-online/src/lib.rs index d8f3fdc854b16..0290c564ec599 100644 --- a/frame/im-online/src/lib.rs +++ b/frame/im-online/src/lib.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! # I'm online Module +//! # I'm online Pallet //! //! If the local node is a validator (i.e. contains an authority key), this module //! gossips a heartbeat transaction with each new session. The heartbeat functions @@ -32,7 +32,7 @@ //! //! - [`Config`] //! - [`Call`] -//! - [`Module`] +//! - [`Pallet`] //! //! ## Interface //! @@ -54,7 +54,7 @@ //! #[weight = 0] //! pub fn is_online(origin, authority_index: u32) -> dispatch::DispatchResult { //! let _sender = ensure_signed(origin)?; -//! let _is_online = >::is_online(authority_index); +//! let _is_online = >::is_online(authority_index); //! Ok(()) //! } //! } @@ -81,27 +81,19 @@ use sp_std::prelude::*; use sp_std::convert::TryInto; use sp_runtime::{ offchain::storage::StorageValueRef, - traits::{AtLeast32BitUnsigned, Convert, Member, Saturating}, - transaction_validity::{ - InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, - ValidTransaction, - }, + traits::{AtLeast32BitUnsigned, Convert, Saturating}, Perbill, Percent, RuntimeDebug, }; use sp_staking::{ SessionIndex, offence::{ReportOffence, Offence, Kind}, }; -use frame_support::{ - decl_error, decl_event, decl_module, decl_storage, - traits::{ - EstimateNextSessionRotation, Get, OneSessionHandler, ValidatorSet, - ValidatorSetWithIdentification, - }, - Parameter, +use frame_support::traits::{ + EstimateNextSessionRotation, OneSessionHandler, ValidatorSet, ValidatorSetWithIdentification, }; -use frame_system::{ensure_none, offchain::{SendTransactionTypes, SubmitTransaction}}; +use frame_system::offchain::{SendTransactionTypes, SubmitTransaction}; pub use weights::WeightInfo; +pub use pallet::*; pub mod sr25519 { mod app_sr25519 { @@ -238,108 +230,152 @@ pub type IdentificationTuple = ( ValidatorSetWithIdentification<::AccountId>>::Identification, ); -pub trait Config: SendTransactionTypes> + frame_system::Config { - /// The identifier type for an authority. - type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + Ord; - - /// The overarching event type. - type Event: From> + Into<::Event>; - - /// A type for retrieving the validators supposed to be online in a session. - type ValidatorSet: ValidatorSetWithIdentification; - - /// A trait that allows us to estimate the current session progress and also the - /// average session length. - /// - /// This parameter is used to determine the longevity of `heartbeat` transaction and a - /// rough time when we should start considering sending heartbeats, since the workers - /// avoids sending them at the very beginning of the session, assuming there is a - /// chance the authority will produce a block and they won't be necessary. - type NextSessionRotation: EstimateNextSessionRotation; - - /// A type that gives us the ability to submit unresponsiveness offence reports. - type ReportUnresponsiveness: ReportOffence< - Self::AccountId, - IdentificationTuple, - UnresponsivenessOffence>, - >; +type OffchainResult = Result::BlockNumber>>; - /// A configuration for base priority of unsigned transactions. - /// - /// This is exposed so that it can be tuned for particular runtime, when - /// multiple pallets send unsigned transactions. - type UnsignedPriority: Get; +#[frame_support::pallet] +pub mod pallet { + use frame_support::{pallet_prelude::*, traits::Get}; + use frame_system::{pallet_prelude::*, ensure_none}; + use sp_runtime::{ + traits::{Member, MaybeSerializeDeserialize}, + transaction_validity::{ + InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, ValidTransaction, + }, + }; + use frame_support::Parameter; + use super::*; + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: SendTransactionTypes> + frame_system::Config { + /// The identifier type for an authority. + type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + Ord + MaybeSerializeDeserialize; + + /// The overarching event type. + type Event: From> + IsType<::Event>; + + /// A type for retrieving the validators supposed to be online in a session. + type ValidatorSet: ValidatorSetWithIdentification; + + /// A trait that allows us to estimate the current session progress and also the + /// average session length. + /// + /// This parameter is used to determine the longevity of `heartbeat` transaction and a + /// rough time when we should start considering sending heartbeats, since the workers + /// avoids sending them at the very beginning of the session, assuming there is a + /// chance the authority will produce a block and they won't be necessary. + type NextSessionRotation: EstimateNextSessionRotation; + + /// A type that gives us the ability to submit unresponsiveness offence reports. + type ReportUnresponsiveness: ReportOffence< + Self::AccountId, + IdentificationTuple, + UnresponsivenessOffence>, + >; + + /// A configuration for base priority of unsigned transactions. + /// + /// This is exposed so that it can be tuned for particular runtime, when + /// multiple pallets send unsigned transactions. + type UnsignedPriority: Get; - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; + } -decl_event!( - pub enum Event where - ::AuthorityId, - IdentificationTuple = IdentificationTuple, - { + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + #[pallet::metadata(T::AuthorityId = "AuthorityId", Vec> = "Vec")] + pub enum Event { /// A new heartbeat was received from `AuthorityId` \[authority_id\] - HeartbeatReceived(AuthorityId), + HeartbeatReceived(T::AuthorityId), /// At the end of the session, no offence was committed. AllGood, /// At the end of the session, at least one validator was found to be \[offline\]. - SomeOffline(Vec), + SomeOffline(Vec>), } -); -decl_storage! { - trait Store for Module as ImOnline { - /// The block number after which it's ok to send heartbeats in the current - /// session. - /// - /// At the beginning of each session we set this to a value that should fall - /// roughly in the middle of the session duration. The idea is to first wait for - /// the validators to produce a block in the current session, so that the - /// heartbeat later on will not be necessary. - /// - /// This value will only be used as a fallback if we fail to get a proper session - /// progress estimate from `NextSessionRotation`, as those estimates should be - /// more accurate then the value we calculate for `HeartbeatAfter`. - HeartbeatAfter get(fn heartbeat_after): T::BlockNumber; - - /// The current set of keys that may issue a heartbeat. - Keys get(fn keys): Vec; - - /// For each session index, we keep a mapping of `AuthIndex` to - /// `offchain::OpaqueNetworkState`. - ReceivedHeartbeats get(fn received_heartbeats): - double_map hasher(twox_64_concat) SessionIndex, hasher(twox_64_concat) AuthIndex - => Option>; - - /// For each session index, we keep a mapping of `ValidatorId` to the - /// number of blocks authored by the given authority. - AuthoredBlocks get(fn authored_blocks): - double_map hasher(twox_64_concat) SessionIndex, hasher(twox_64_concat) ValidatorId - => u32; - } - add_extra_genesis { - config(keys): Vec; - build(|config| Module::::initialize_keys(&config.keys)) - } -} - -decl_error! { - /// Error for the im-online module. - pub enum Error for Module { + #[pallet::error] + pub enum Error { /// Non existent public key. InvalidKey, /// Duplicated heartbeat. DuplicatedHeartbeat, } -} -decl_module! { - pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + /// The block number after which it's ok to send heartbeats in the current + /// session. + /// + /// At the beginning of each session we set this to a value that should fall + /// roughly in the middle of the session duration. The idea is to first wait for + /// the validators to produce a block in the current session, so that the + /// heartbeat later on will not be necessary. + /// + /// This value will only be used as a fallback if we fail to get a proper session + /// progress estimate from `NextSessionRotation`, as those estimates should be + /// more accurate then the value we calculate for `HeartbeatAfter`. + #[pallet::storage] + #[pallet::getter(fn heartbeat_after)] + pub(crate) type HeartbeatAfter = StorageValue<_, T::BlockNumber, ValueQuery>; + + /// The current set of keys that may issue a heartbeat. + #[pallet::storage] + #[pallet::getter(fn keys)] + pub(crate) type Keys = StorageValue<_, Vec, ValueQuery>; + + /// For each session index, we keep a mapping of `AuthIndex` to + /// `offchain::OpaqueNetworkState`. + #[pallet::storage] + #[pallet::getter(fn received_heartbeats)] + pub(crate) type ReceivedHeartbeats = StorageDoubleMap< + _, + Twox64Concat, + SessionIndex, + Twox64Concat, + AuthIndex, + Vec, + >; + + /// For each session index, we keep a mapping of `ValidatorId` to the + /// number of blocks authored by the given authority. + #[pallet::storage] + #[pallet::getter(fn authored_blocks)] + pub(crate) type AuthoredBlocks = StorageDoubleMap< + _, + Twox64Concat, + SessionIndex, + Twox64Concat, + ValidatorId, + u32, + ValueQuery, + >; + + #[pallet::genesis_config] + pub struct GenesisConfig { + pub keys: Vec, + } + + #[cfg(feature = "std")] + impl Default for GenesisConfig { + fn default() -> Self { + GenesisConfig { + keys: Default::default(), + } + } + } - fn deposit_event() = default; + #[pallet::genesis_build] + impl GenesisBuild for GenesisConfig { + fn build(&self) { + Pallet::::initialize_keys(&self.keys); + } + } + #[pallet::call] + impl Pallet { /// # /// - Complexity: `O(K + E)` where K is length of `Keys` (heartbeat.validators_len) /// and E is length of `heartbeat.network_state.external_address` @@ -351,21 +387,21 @@ decl_module! { /// # // NOTE: the weight includes the cost of validate_unsigned as it is part of the cost to // import block with such an extrinsic. - #[weight = ::WeightInfo::validate_unsigned_and_then_heartbeat( + #[pallet::weight(::WeightInfo::validate_unsigned_and_then_heartbeat( heartbeat.validators_len as u32, heartbeat.network_state.external_addresses.len() as u32, - )] - fn heartbeat( - origin, + ))] + pub fn heartbeat( + origin: OriginFor, heartbeat: Heartbeat, // since signature verification is done in `validate_unsigned` // we can skip doing it here again. _signature: ::Signature, - ) { + ) -> DispatchResultWithPostInfo { ensure_none(origin)?; let current_session = T::ValidatorSet::session_index(); - let exists = ::contains_key( + let exists = ReceivedHeartbeats::::contains_key( ¤t_session, &heartbeat.authority_index ); @@ -375,20 +411,24 @@ decl_module! { Self::deposit_event(Event::::HeartbeatReceived(public.clone())); let network_state = heartbeat.network_state.encode(); - ::insert( + ReceivedHeartbeats::::insert( ¤t_session, &heartbeat.authority_index, &network_state ); + + Ok(().into()) } else if exists { Err(Error::::DuplicatedHeartbeat)? } else { Err(Error::::InvalidKey)? } } + } - // Runs after every block. - fn offchain_worker(now: T::BlockNumber) { + #[pallet::hooks] + impl Hooks> for Pallet { + fn offchain_worker(now: BlockNumberFor) { // Only send messages if we are a potential validator. if sp_io::offchain::is_validator() { for res in Self::send_heartbeats(now).into_iter().flatten() { @@ -410,15 +450,69 @@ decl_module! { } } } -} -type OffchainResult = Result::BlockNumber>>; + /// Invalid transaction custom error. Returned when validators_len field in heartbeat is incorrect. + pub(crate) const INVALID_VALIDATORS_LEN: u8 = 10; + + #[pallet::validate_unsigned] + impl ValidateUnsigned for Pallet { + type Call = Call; + + fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { + if let Call::heartbeat(heartbeat, signature) = call { + if >::is_online(heartbeat.authority_index) { + // we already received a heartbeat for this authority + return InvalidTransaction::Stale.into(); + } + + // check if session index from heartbeat is recent + let current_session = T::ValidatorSet::session_index(); + if heartbeat.session_index != current_session { + return InvalidTransaction::Stale.into(); + } + + // verify that the incoming (unverified) pubkey is actually an authority id + let keys = Keys::::get(); + if keys.len() as u32 != heartbeat.validators_len { + return InvalidTransaction::Custom(INVALID_VALIDATORS_LEN).into(); + } + let authority_id = match keys.get(heartbeat.authority_index as usize) { + Some(id) => id, + None => return InvalidTransaction::BadProof.into(), + }; + + // check signature (this is expensive so we do it last). + let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| { + authority_id.verify(&encoded_heartbeat, &signature) + }); + + if !signature_valid { + return InvalidTransaction::BadProof.into(); + } + + ValidTransaction::with_tag_prefix("ImOnline") + .priority(T::UnsignedPriority::get()) + .and_provides((current_session, authority_id)) + .longevity( + TryInto::::try_into( + T::NextSessionRotation::average_session_length() / 2u32.into(), + ) + .unwrap_or(64_u64), + ) + .propagate(true) + .build() + } else { + InvalidTransaction::Call.into() + } + } + } +} /// Keep track of number of authored blocks per authority, uncles are counted as /// well since they're a valid proof of being online. impl< T: Config + pallet_authorship::Config, -> pallet_authorship::EventHandler, T::BlockNumber> for Module +> pallet_authorship::EventHandler, T::BlockNumber> for Pallet { fn note_author(author: ValidatorId) { Self::note_authorship(author); @@ -429,7 +523,7 @@ impl< } } -impl Module { +impl Pallet { /// Returns `true` if a heartbeat has been received for the authority at /// `authority_index` in the authorities series or if the authority has /// authored at least one block, during the current session. Otherwise @@ -449,8 +543,8 @@ impl Module { fn is_online_aux(authority_index: AuthIndex, authority: &ValidatorId) -> bool { let current_session = T::ValidatorSet::session_index(); - ::contains_key(¤t_session, &authority_index) || - >::get( + ReceivedHeartbeats::::contains_key(¤t_session, &authority_index) || + AuthoredBlocks::::get( ¤t_session, authority, ) != 0 @@ -460,14 +554,14 @@ impl Module { /// the authorities series, during the current session. Otherwise `false`. pub fn received_heartbeat_in_current_session(authority_index: AuthIndex) -> bool { let current_session = T::ValidatorSet::session_index(); - ::contains_key(¤t_session, &authority_index) + ReceivedHeartbeats::::contains_key(¤t_session, &authority_index) } /// Note that the given authority has authored a block in the current session. fn note_authorship(author: ValidatorId) { let current_session = T::ValidatorSet::session_index(); - >::mutate( + AuthoredBlocks::::mutate( ¤t_session, author, |authored| *authored += 1, @@ -648,11 +742,11 @@ impl Module { } } -impl sp_runtime::BoundToRuntimeAppPublic for Module { +impl sp_runtime::BoundToRuntimeAppPublic for Pallet { type Public = T::AuthorityId; } -impl OneSessionHandler for Module { +impl OneSessionHandler for Pallet { type Key = T::AuthorityId; fn on_genesis_session<'a, I: 'a>(validators: I) @@ -693,13 +787,13 @@ impl OneSessionHandler for Module { // Remove all received heartbeats and number of authored blocks from the // current session, they have already been processed and won't be needed // anymore. - ::remove_prefix(&T::ValidatorSet::session_index()); - >::remove_prefix(&T::ValidatorSet::session_index()); + ReceivedHeartbeats::::remove_prefix(&T::ValidatorSet::session_index()); + AuthoredBlocks::::remove_prefix(&T::ValidatorSet::session_index()); if offenders.is_empty() { - Self::deposit_event(RawEvent::AllGood); + Self::deposit_event(Event::::AllGood); } else { - Self::deposit_event(RawEvent::SomeOffline(offenders.clone())); + Self::deposit_event(Event::::SomeOffline(offenders.clone())); let validator_set_count = keys.len() as u32; let offence = UnresponsivenessOffence { session_index, validator_set_count, offenders }; @@ -714,61 +808,6 @@ impl OneSessionHandler for Module { } } -/// Invalid transaction custom error. Returned when validators_len field in heartbeat is incorrect. -const INVALID_VALIDATORS_LEN: u8 = 10; - -impl frame_support::unsigned::ValidateUnsigned for Module { - type Call = Call; - - fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { - if let Call::heartbeat(heartbeat, signature) = call { - if >::is_online(heartbeat.authority_index) { - // we already received a heartbeat for this authority - return InvalidTransaction::Stale.into(); - } - - // check if session index from heartbeat is recent - let current_session = T::ValidatorSet::session_index(); - if heartbeat.session_index != current_session { - return InvalidTransaction::Stale.into(); - } - - // verify that the incoming (unverified) pubkey is actually an authority id - let keys = Keys::::get(); - if keys.len() as u32 != heartbeat.validators_len { - return InvalidTransaction::Custom(INVALID_VALIDATORS_LEN).into(); - } - let authority_id = match keys.get(heartbeat.authority_index as usize) { - Some(id) => id, - None => return InvalidTransaction::BadProof.into(), - }; - - // check signature (this is expensive so we do it last). - let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| { - authority_id.verify(&encoded_heartbeat, &signature) - }); - - if !signature_valid { - return InvalidTransaction::BadProof.into(); - } - - ValidTransaction::with_tag_prefix("ImOnline") - .priority(T::UnsignedPriority::get()) - .and_provides((current_session, authority_id)) - .longevity( - TryInto::::try_into( - T::NextSessionRotation::average_session_length() / 2u32.into(), - ) - .unwrap_or(64_u64), - ) - .propagate(true) - .build() - } else { - InvalidTransaction::Call.into() - } - } -} - /// An offence that is filed if a validator didn't send a heartbeat message. #[derive(RuntimeDebug)] #[cfg_attr(feature = "std", derive(Clone, PartialEq, Eq))] diff --git a/frame/im-online/src/tests.rs b/frame/im-online/src/tests.rs index f447a2ade5481..5ce931875b9a6 100644 --- a/frame/im-online/src/tests.rs +++ b/frame/im-online/src/tests.rs @@ -29,7 +29,7 @@ use sp_core::offchain::{ testing::{TestOffchainExt, TestTransactionPoolExt}, }; use frame_support::{dispatch, assert_noop}; -use sp_runtime::{testing::UintAuthorityId, transaction_validity::TransactionValidityError}; +use sp_runtime::{testing::UintAuthorityId, transaction_validity::{TransactionValidityError, InvalidTransaction}}; #[test] fn test_unresponsiveness_slash_fraction() { @@ -114,7 +114,7 @@ fn heartbeat( authority_index: u32, id: UintAuthorityId, validators: Vec, -) -> dispatch::DispatchResult { +) -> dispatch::DispatchResultWithPostInfo { use frame_support::unsigned::ValidateUnsigned; let heartbeat = Heartbeat { diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 4e5160c6673fa..f65bdddd36d02 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -37,7 +37,7 @@ use sp_staking::offence::{ReportOffence, Offence}; use pallet_balances::Config as BalancesConfig; use pallet_babe::BabeEquivocationOffence; use pallet_grandpa::{GrandpaEquivocationOffence, GrandpaTimeSlot}; -use pallet_im_online::{Config as ImOnlineConfig, Module as ImOnline, UnresponsivenessOffence}; +use pallet_im_online::{Config as ImOnlineConfig, Pallet as ImOnline, UnresponsivenessOffence}; use pallet_offences::{Config as OffencesConfig, Module as Offences}; use pallet_session::historical::{Config as HistoricalConfig, IdentificationTuple}; use pallet_session::{Config as SessionConfig, SessionManager};