diff --git a/substrate/frame/im-online/src/benchmarking.rs b/substrate/frame/im-online/src/benchmarking.rs index 287a2c6fd3a73e1004be65e5f60c5702c76c6641..5ab4d16c7fe087c713b0da31331aa51eaaf151f6 100644 --- a/substrate/frame/im-online/src/benchmarking.rs +++ b/substrate/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/substrate/frame/im-online/src/lib.rs b/substrate/frame/im-online/src/lib.rs index d8f3fdc854b165b4b53762ddfdb015e904df165e..0290c564ec599314c2a5f9ba6105b177472d9e89 100644 --- a/substrate/frame/im-online/src/lib.rs +++ b/substrate/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 = <im_online::Module<T>>::is_online(authority_index); +//! let _is_online = <im_online::Pallet<T>>::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<T> = ( ValidatorSetWithIdentification<<T as frame_system::Config>::AccountId>>::Identification, ); -pub trait Config: SendTransactionTypes<Call<Self>> + frame_system::Config { - /// The identifier type for an authority. - type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + Ord; - - /// The overarching event type. - type Event: From<Event<Self>> + Into<<Self as frame_system::Config>::Event>; - - /// A type for retrieving the validators supposed to be online in a session. - type ValidatorSet: ValidatorSetWithIdentification<Self::AccountId>; - - /// 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<Self::BlockNumber>; - - /// A type that gives us the ability to submit unresponsiveness offence reports. - type ReportUnresponsiveness: ReportOffence< - Self::AccountId, - IdentificationTuple<Self>, - UnresponsivenessOffence<IdentificationTuple<Self>>, - >; +type OffchainResult<T, A> = Result<A, OffchainErr<<T as frame_system::Config>::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<TransactionPriority>; +#[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<T>(_); + + #[pallet::config] + pub trait Config: SendTransactionTypes<Call<Self>> + frame_system::Config { + /// The identifier type for an authority. + type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + Ord + MaybeSerializeDeserialize; + + /// The overarching event type. + type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>; + + /// A type for retrieving the validators supposed to be online in a session. + type ValidatorSet: ValidatorSetWithIdentification<Self::AccountId>; + + /// 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<Self::BlockNumber>; + + /// A type that gives us the ability to submit unresponsiveness offence reports. + type ReportUnresponsiveness: ReportOffence< + Self::AccountId, + IdentificationTuple<Self>, + UnresponsivenessOffence<IdentificationTuple<Self>>, + >; + + /// 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<TransactionPriority>; - /// 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<T> where - <T as Config>::AuthorityId, - IdentificationTuple = IdentificationTuple<T>, - { + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + #[pallet::metadata(T::AuthorityId = "AuthorityId", Vec<IdentificationTuple<T>> = "Vec<IdentificationTuple>")] + pub enum Event<T: Config> { /// 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<IdentificationTuple>), + SomeOffline(Vec<IdentificationTuple<T>>), } -); -decl_storage! { - trait Store for Module<T: Config> 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<T::AuthorityId>; - - /// 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<Vec<u8>>; - - /// For each session index, we keep a mapping of `ValidatorId<T>` 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<T> - => u32; - } - add_extra_genesis { - config(keys): Vec<T::AuthorityId>; - build(|config| Module::<T>::initialize_keys(&config.keys)) - } -} - -decl_error! { - /// Error for the im-online module. - pub enum Error for Module<T: Config> { + #[pallet::error] + pub enum Error<T> { /// Non existent public key. InvalidKey, /// Duplicated heartbeat. DuplicatedHeartbeat, } -} -decl_module! { - pub struct Module<T: Config> for enum Call where origin: T::Origin { - type Error = Error<T>; + /// 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<T: Config> = StorageValue<_, T::BlockNumber, ValueQuery>; + + /// The current set of keys that may issue a heartbeat. + #[pallet::storage] + #[pallet::getter(fn keys)] + pub(crate) type Keys<T: Config> = StorageValue<_, Vec<T::AuthorityId>, 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<T> = StorageDoubleMap< + _, + Twox64Concat, + SessionIndex, + Twox64Concat, + AuthIndex, + Vec<u8>, + >; + + /// For each session index, we keep a mapping of `ValidatorId<T>` to the + /// number of blocks authored by the given authority. + #[pallet::storage] + #[pallet::getter(fn authored_blocks)] + pub(crate) type AuthoredBlocks<T: Config> = StorageDoubleMap< + _, + Twox64Concat, + SessionIndex, + Twox64Concat, + ValidatorId<T>, + u32, + ValueQuery, + >; + + #[pallet::genesis_config] + pub struct GenesisConfig<T: Config> { + pub keys: Vec<T::AuthorityId>, + } + + #[cfg(feature = "std")] + impl<T: Config> Default for GenesisConfig<T> { + fn default() -> Self { + GenesisConfig { + keys: Default::default(), + } + } + } - fn deposit_event() = default; + #[pallet::genesis_build] + impl<T: Config> GenesisBuild<T> for GenesisConfig<T> { + fn build(&self) { + Pallet::<T>::initialize_keys(&self.keys); + } + } + #[pallet::call] + impl<T: Config> Pallet<T> { /// # <weight> /// - 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! { /// # </weight> // NOTE: the weight includes the cost of validate_unsigned as it is part of the cost to // import block with such an extrinsic. - #[weight = <T as Config>::WeightInfo::validate_unsigned_and_then_heartbeat( + #[pallet::weight(<T as Config>::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<T>, heartbeat: Heartbeat<T::BlockNumber>, // since signature verification is done in `validate_unsigned` // we can skip doing it here again. _signature: <T::AuthorityId as RuntimeAppPublic>::Signature, - ) { + ) -> DispatchResultWithPostInfo { ensure_none(origin)?; let current_session = T::ValidatorSet::session_index(); - let exists = <ReceivedHeartbeats>::contains_key( + let exists = ReceivedHeartbeats::<T>::contains_key( ¤t_session, &heartbeat.authority_index ); @@ -375,20 +411,24 @@ decl_module! { Self::deposit_event(Event::<T>::HeartbeatReceived(public.clone())); let network_state = heartbeat.network_state.encode(); - <ReceivedHeartbeats>::insert( + ReceivedHeartbeats::<T>::insert( ¤t_session, &heartbeat.authority_index, &network_state ); + + Ok(().into()) } else if exists { Err(Error::<T>::DuplicatedHeartbeat)? } else { Err(Error::<T>::InvalidKey)? } } + } - // Runs after every block. - fn offchain_worker(now: T::BlockNumber) { + #[pallet::hooks] + impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> { + fn offchain_worker(now: BlockNumberFor<T>) { // 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<T, A> = Result<A, OffchainErr<<T as frame_system::Config>::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<T: Config> ValidateUnsigned for Pallet<T> { + type Call = Call<T>; + + fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { + if let Call::heartbeat(heartbeat, signature) = call { + if <Pallet<T>>::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::<T>::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::<u64>::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<ValidatorId<T>, T::BlockNumber> for Module<T> +> pallet_authorship::EventHandler<ValidatorId<T>, T::BlockNumber> for Pallet<T> { fn note_author(author: ValidatorId<T>) { Self::note_authorship(author); @@ -429,7 +523,7 @@ impl< } } -impl<T: Config> Module<T> { +impl<T: Config> Pallet<T> { /// 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<T: Config> Module<T> { fn is_online_aux(authority_index: AuthIndex, authority: &ValidatorId<T>) -> bool { let current_session = T::ValidatorSet::session_index(); - <ReceivedHeartbeats>::contains_key(¤t_session, &authority_index) || - <AuthoredBlocks<T>>::get( + ReceivedHeartbeats::<T>::contains_key(¤t_session, &authority_index) || + AuthoredBlocks::<T>::get( ¤t_session, authority, ) != 0 @@ -460,14 +554,14 @@ impl<T: Config> Module<T> { /// 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(); - <ReceivedHeartbeats>::contains_key(¤t_session, &authority_index) + ReceivedHeartbeats::<T>::contains_key(¤t_session, &authority_index) } /// Note that the given authority has authored a block in the current session. fn note_authorship(author: ValidatorId<T>) { let current_session = T::ValidatorSet::session_index(); - <AuthoredBlocks<T>>::mutate( + AuthoredBlocks::<T>::mutate( ¤t_session, author, |authored| *authored += 1, @@ -648,11 +742,11 @@ impl<T: Config> Module<T> { } } -impl<T: Config> sp_runtime::BoundToRuntimeAppPublic for Module<T> { +impl<T: Config> sp_runtime::BoundToRuntimeAppPublic for Pallet<T> { type Public = T::AuthorityId; } -impl<T: Config> OneSessionHandler<T::AccountId> for Module<T> { +impl<T: Config> OneSessionHandler<T::AccountId> for Pallet<T> { type Key = T::AuthorityId; fn on_genesis_session<'a, I: 'a>(validators: I) @@ -693,13 +787,13 @@ impl<T: Config> OneSessionHandler<T::AccountId> for Module<T> { // Remove all received heartbeats and number of authored blocks from the // current session, they have already been processed and won't be needed // anymore. - <ReceivedHeartbeats>::remove_prefix(&T::ValidatorSet::session_index()); - <AuthoredBlocks<T>>::remove_prefix(&T::ValidatorSet::session_index()); + ReceivedHeartbeats::<T>::remove_prefix(&T::ValidatorSet::session_index()); + AuthoredBlocks::<T>::remove_prefix(&T::ValidatorSet::session_index()); if offenders.is_empty() { - Self::deposit_event(RawEvent::AllGood); + Self::deposit_event(Event::<T>::AllGood); } else { - Self::deposit_event(RawEvent::SomeOffline(offenders.clone())); + Self::deposit_event(Event::<T>::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<T: Config> OneSessionHandler<T::AccountId> for Module<T> { } } -/// Invalid transaction custom error. Returned when validators_len field in heartbeat is incorrect. -const INVALID_VALIDATORS_LEN: u8 = 10; - -impl<T: Config> frame_support::unsigned::ValidateUnsigned for Module<T> { - type Call = Call<T>; - - fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { - if let Call::heartbeat(heartbeat, signature) = call { - if <Module<T>>::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::<T>::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::<u64>::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/substrate/frame/im-online/src/tests.rs b/substrate/frame/im-online/src/tests.rs index f447a2ade54819f7f9bdae66964de8cd463a15d8..5ce931875b9a67d391ad00c421b83f5807b11672 100644 --- a/substrate/frame/im-online/src/tests.rs +++ b/substrate/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<u64>, -) -> dispatch::DispatchResult { +) -> dispatch::DispatchResultWithPostInfo { use frame_support::unsigned::ValidateUnsigned; let heartbeat = Heartbeat { diff --git a/substrate/frame/offences/benchmarking/src/lib.rs b/substrate/frame/offences/benchmarking/src/lib.rs index 4e5160c6673fa893161363c39662a36be5c67922..f65bdddd36d02cc83f25cb76f4b64da1acf3245c 100644 --- a/substrate/frame/offences/benchmarking/src/lib.rs +++ b/substrate/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};