Migrate pallet-im-online to pallet attribute macro. (#8714)

* Migrate pallet-im-online to pallet attribute macro.

* Move validate_unsigned into pallet macro.

* fix metadata

* fix test

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

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;

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;
+type OffchainResult<T, A> = Result<A, OffchainErr<<T as frame_system::Config>::BlockNumber>>;
 
-	/// The overarching event type.
-	type Event: From<Event<Self>> + Into<<Self as frame_system::Config>::Event>;
+#[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::*;
 
-	/// A type for retrieving the validators supposed to be online in a session.
-	type ValidatorSet: ValidatorSetWithIdentification<Self::AccountId>;
+	#[pallet::pallet]
+	#[pallet::generate_store(pub(super) trait Store)]
+	pub struct Pallet<T>(_);
 
-	/// 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>;
+	#[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;
 
-	/// A type that gives us the ability to submit unresponsiveness offence reports.
-	type ReportUnresponsiveness: ReportOffence<
-		Self::AccountId,
-		IdentificationTuple<Self>,
-		UnresponsivenessOffence<IdentificationTuple<Self>>,
-	>;
+		/// The overarching event type.
+		type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
 
-	/// 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>;
+		/// A type for retrieving the validators supposed to be online in a session.
+		type ValidatorSet: ValidatorSetWithIdentification<Self::AccountId>;
 
-	/// Weight information for extrinsics in this pallet.
-	type WeightInfo: WeightInfo;
-}
+		/// 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>;
 
-decl_event!(
-	pub enum Event<T> where
-		<T as Config>::AuthorityId,
-		IdentificationTuple = IdentificationTuple<T>,
-	{
+		/// 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;
+	}
+
+	#[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>;
 
-		fn deposit_event() = default;
+	/// 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(),
+			}
+		}
+	}
+
+	#[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(
 				&current_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(
 					&current_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(&current_session, &authority_index) ||
-			<AuthoredBlocks<T>>::get(
+		ReceivedHeartbeats::<T>::contains_key(&current_session, &authority_index) ||
+			AuthoredBlocks::<T>::get(
 				&current_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(&current_session, &authority_index)
+		ReceivedHeartbeats::<T>::contains_key(&current_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(
 			&current_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))]

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 {