From e0f85464811a86b06bc0ee071167bd45d120f156 Mon Sep 17 00:00:00 2001 From: ferrell-code <70108835+ferrell-code@users.noreply.github.com> Date: Sun, 2 May 2021 22:43:56 -0400 Subject: [PATCH] Upgrade authorship pallet to Frame-v2 (#8663) * first commit * get to compile * fix deprecated grandpa * formatting * module to pallet * add authorship pallet to mocks * Fix upgrade of storage. Co-authored-by: Xiliang Chen <xlchen1291@gmail.com> * trigger CI * put back doc Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> Co-authored-by: Xiliang Chen <xlchen1291@gmail.com> --- substrate/frame/authorship/src/lib.rs | 171 +++++++++++--------- substrate/frame/babe/src/equivocation.rs | 2 +- substrate/frame/babe/src/mock.rs | 1 + substrate/frame/grandpa/src/equivocation.rs | 2 +- substrate/frame/grandpa/src/mock.rs | 1 + substrate/frame/staking/src/lib.rs | 2 +- substrate/frame/staking/src/mock.rs | 1 + substrate/frame/staking/src/tests.rs | 2 +- 8 files changed, 101 insertions(+), 81 deletions(-) diff --git a/substrate/frame/authorship/src/lib.rs b/substrate/frame/authorship/src/lib.rs index a7803319c53..4243ae55718 100644 --- a/substrate/frame/authorship/src/lib.rs +++ b/substrate/frame/authorship/src/lib.rs @@ -23,43 +23,18 @@ use sp_std::{result, prelude::*}; use sp_std::collections::btree_set::BTreeSet; -use frame_support::{decl_module, decl_storage, decl_error, dispatch, ensure}; +use frame_support::dispatch; use frame_support::traits::{FindAuthor, VerifySeal, Get}; use codec::{Encode, Decode}; -use frame_system::ensure_none; use sp_runtime::traits::{Header as HeaderT, One, Zero}; -use frame_support::weights::{Weight, DispatchClass}; use sp_inherents::{InherentIdentifier, ProvideInherent, InherentData}; use sp_authorship::{INHERENT_IDENTIFIER, UnclesInherentData, InherentError}; const MAX_UNCLES: usize = 10; -pub trait Config: frame_system::Config { - /// Find the author of a block. - type FindAuthor: FindAuthor<Self::AccountId>; - /// The number of blocks back we should accept uncles. - /// This means that we will deal with uncle-parents that are - /// `UncleGenerations + 1` before `now`. - type UncleGenerations: Get<Self::BlockNumber>; - /// A filter for uncles within a block. This is for implementing - /// further constraints on what uncles can be included, other than their ancestry. - /// - /// For PoW, as long as the seals are checked, there is no need to use anything - /// but the `VerifySeal` implementation as the filter. This is because the cost of making many equivocating - /// uncles is high. - /// - /// For PoS, there is no such limitation, so a further constraint must be imposed - /// beyond a seal check in order to prevent an arbitrary number of - /// equivocating uncles from being included. - /// - /// The `OnePerAuthorPerHeight` filter is good for many slot-based PoS - /// engines. - type FilterUncle: FilterUncle<Self::Header, Self::AccountId>; - /// An event handler for authored blocks. - type EventHandler: EventHandler<Self::AccountId, Self::BlockNumber>; -} +pub use pallet::*; -/// An event handler for the authorship module. There is a dummy implementation +/// An event handler for the authorship pallet. There is a dummy implementation /// for `()`, which does nothing. #[impl_trait_for_tuples::impl_for_tuples(30)] pub trait EventHandler<Author, BlockNumber> { @@ -150,41 +125,45 @@ enum UncleEntryItem<BlockNumber, Hash, Author> { InclusionHeight(BlockNumber), Uncle(Hash, Option<Author>), } +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use super::*; -decl_storage! { - trait Store for Module<T: Config> as Authorship { - /// Uncles - Uncles: Vec<UncleEntryItem<T::BlockNumber, T::Hash, T::AccountId>>; - /// Author of current block. - Author: Option<T::AccountId>; - /// Whether uncles were already set in this block. - DidSetUncles: bool; + #[pallet::config] + pub trait Config: frame_system::Config { + /// Find the author of a block. + type FindAuthor: FindAuthor<Self::AccountId>; + /// The number of blocks back we should accept uncles. + /// This means that we will deal with uncle-parents that are + /// `UncleGenerations + 1` before `now`. + type UncleGenerations: Get<Self::BlockNumber>; + /// A filter for uncles within a block. This is for implementing + /// further constraints on what uncles can be included, other than their ancestry. + /// + /// For PoW, as long as the seals are checked, there is no need to use anything + /// but the `VerifySeal` implementation as the filter. This is because the cost of making many equivocating + /// uncles is high. + /// + /// For PoS, there is no such limitation, so a further constraint must be imposed + /// beyond a seal check in order to prevent an arbitrary number of + /// equivocating uncles from being included. + /// + /// The `OnePerAuthorPerHeight` filter is good for many slot-based PoS + /// engines. + type FilterUncle: FilterUncle<Self::Header, Self::AccountId>; + /// An event handler for authored blocks. + type EventHandler: EventHandler<Self::AccountId, Self::BlockNumber>; } -} -decl_error! { - /// Error for the authorship module. - pub enum Error for Module<T: Config> { - /// The uncle parent not in the chain. - InvalidUncleParent, - /// Uncles already set in the block. - UnclesAlreadySet, - /// Too many uncles. - TooManyUncles, - /// The uncle is genesis. - GenesisUncle, - /// The uncle is too high in chain. - TooHighUncle, - /// The uncle is already included. - UncleAlreadyIncluded, - /// The uncle isn't recent enough to be included. - OldUncle, - } -} + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet<T>(_); -decl_module! { - pub struct Module<T: Config> for enum Call where origin: T::Origin { - type Error = Error<T>; + + #[pallet::hooks] + impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> { fn on_initialize(now: T::BlockNumber) -> Weight { let uncle_generations = T::UncleGenerations::get(); @@ -194,50 +173,88 @@ decl_module! { Self::prune_old_uncles(minimum_height) } - <Self as Store>::DidSetUncles::put(false); + <DidSetUncles<T>>::put(false); T::EventHandler::note_author(Self::author()); 0 } - fn on_finalize() { + fn on_finalize(_: T::BlockNumber) { // ensure we never go to trie with these values. - <Self as Store>::Author::kill(); - <Self as Store>::DidSetUncles::kill(); + <Author<T>>::kill(); + <DidSetUncles<T>>::kill(); } + } + + #[pallet::storage] + /// Uncles + pub(super) type Uncles<T: Config> = StorageValue< + _, + Vec<UncleEntryItem<T::BlockNumber, T::Hash, T::AccountId>>, + ValueQuery, + >; + + #[pallet::storage] + /// Author of current block. + pub(super) type Author<T: Config> = StorageValue<_, T::AccountId, OptionQuery>; + + #[pallet::storage] + /// Whether uncles were already set in this block. + pub(super) type DidSetUncles<T: Config> = StorageValue<_, bool, ValueQuery>; + + + #[pallet::error] + pub enum Error<T> { + /// The uncle parent not in the chain. + InvalidUncleParent, + /// Uncles already set in the block. + UnclesAlreadySet, + /// Too many uncles. + TooManyUncles, + /// The uncle is genesis. + GenesisUncle, + /// The uncle is too high in chain. + TooHighUncle, + /// The uncle is already included. + UncleAlreadyIncluded, + /// The uncle isn't recent enough to be included. + OldUncle, + } + #[pallet::call] + impl<T: Config> Pallet<T> { /// Provide a set of uncles. - #[weight = (0, DispatchClass::Mandatory)] - fn set_uncles(origin, new_uncles: Vec<T::Header>) -> dispatch::DispatchResult { + #[pallet::weight((0, DispatchClass::Mandatory))] + fn set_uncles(origin: OriginFor<T>, new_uncles: Vec<T::Header>) -> DispatchResult { ensure_none(origin)?; ensure!(new_uncles.len() <= MAX_UNCLES, Error::<T>::TooManyUncles); - if <Self as Store>::DidSetUncles::get() { + if <DidSetUncles<T>>::get() { Err(Error::<T>::UnclesAlreadySet)? } - <Self as Store>::DidSetUncles::put(true); + <DidSetUncles<T>>::put(true); Self::verify_and_import_uncles(new_uncles) } } } -impl<T: Config> Module<T> { +impl<T: Config> Pallet<T> { /// Fetch the author of the block. /// /// This is safe to invoke in `on_initialize` implementations, as well /// as afterwards. pub fn author() -> T::AccountId { // Check the memoized storage value. - if let Some(author) = <Self as Store>::Author::get() { + if let Some(author) = <Author<T>>::get() { return author; } let digest = <frame_system::Pallet<T>>::digest(); let pre_runtime_digests = digest.logs.iter().filter_map(|d| d.as_pre_runtime()); if let Some(author) = T::FindAuthor::find_author(pre_runtime_digests) { - <Self as Store>::Author::put(&author); + <Author<T>>::put(&author); author } else { Default::default() @@ -247,7 +264,7 @@ impl<T: Config> Module<T> { fn verify_and_import_uncles(new_uncles: Vec<T::Header>) -> dispatch::DispatchResult { let now = <frame_system::Pallet<T>>::block_number(); - let mut uncles = <Self as Store>::Uncles::get(); + let mut uncles = <Uncles<T>>::get(); uncles.push(UncleEntryItem::InclusionHeight(now)); let mut acc: <T::FilterUncle as FilterUncle<_, _>>::Accumulator = Default::default(); @@ -268,7 +285,7 @@ impl<T: Config> Module<T> { uncles.push(UncleEntryItem::Uncle(hash, author)); } - <Self as Store>::Uncles::put(&uncles); + <Uncles<T>>::put(&uncles); Ok(()) } @@ -325,7 +342,7 @@ impl<T: Config> Module<T> { } fn prune_old_uncles(minimum_height: T::BlockNumber) { - let mut uncles = <Self as Store>::Uncles::get(); + let mut uncles = <Uncles<T>>::get(); let prune_entries = uncles.iter().take_while(|item| match item { UncleEntryItem::Uncle(_, _) => true, UncleEntryItem::InclusionHeight(height) => height < &minimum_height, @@ -333,11 +350,11 @@ impl<T: Config> Module<T> { let prune_index = prune_entries.count(); let _ = uncles.drain(..prune_index); - <Self as Store>::Uncles::put(uncles); + <Uncles<T>>::put(uncles); } } -impl<T: Config> ProvideInherent for Module<T> { +impl<T: Config> ProvideInherent for Pallet<T> { type Call = Call<T>; type Error = InherentError; const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER; @@ -347,7 +364,7 @@ impl<T: Config> ProvideInherent for Module<T> { let mut set_uncles = Vec::new(); if !uncles.is_empty() { - let prev_uncles = <Self as Store>::Uncles::get(); + let prev_uncles = <Uncles<T>>::get(); let mut existing_hashes: Vec<_> = prev_uncles.into_iter().filter_map(|entry| match entry { UncleEntryItem::InclusionHeight(_) => None, @@ -458,7 +475,7 @@ mod tests { pub const UncleGenerations: u64 = 5; } - impl Config for Test { + impl pallet::Config for Test { type FindAuthor = AuthorGiven; type UncleGenerations = UncleGenerations; type FilterUncle = SealVerify<VerifyBlock>; diff --git a/substrate/frame/babe/src/equivocation.rs b/substrate/frame/babe/src/equivocation.rs index 154faa49f0b..0fd74882c1b 100644 --- a/substrate/frame/babe/src/equivocation.rs +++ b/substrate/frame/babe/src/equivocation.rs @@ -175,7 +175,7 @@ where } fn block_author() -> Option<T::AccountId> { - Some(<pallet_authorship::Module<T>>::author()) + Some(<pallet_authorship::Pallet<T>>::author()) } } diff --git a/substrate/frame/babe/src/mock.rs b/substrate/frame/babe/src/mock.rs index 39831eceb75..d01a67f4039 100644 --- a/substrate/frame/babe/src/mock.rs +++ b/substrate/frame/babe/src/mock.rs @@ -52,6 +52,7 @@ frame_support::construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event<T>}, + Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, Balances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>}, Historical: pallet_session_historical::{Pallet}, Offences: pallet_offences::{Pallet, Call, Storage, Event}, diff --git a/substrate/frame/grandpa/src/equivocation.rs b/substrate/frame/grandpa/src/equivocation.rs index 8ab86b2fed0..441311ebc54 100644 --- a/substrate/frame/grandpa/src/equivocation.rs +++ b/substrate/frame/grandpa/src/equivocation.rs @@ -186,7 +186,7 @@ where } fn block_author() -> Option<T::AccountId> { - Some(<pallet_authorship::Module<T>>::author()) + Some(<pallet_authorship::Pallet<T>>::author()) } } diff --git a/substrate/frame/grandpa/src/mock.rs b/substrate/frame/grandpa/src/mock.rs index d59d0d19d0e..b13c431dc5b 100644 --- a/substrate/frame/grandpa/src/mock.rs +++ b/substrate/frame/grandpa/src/mock.rs @@ -52,6 +52,7 @@ frame_support::construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event<T>}, + Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, Balances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>}, Staking: pallet_staking::{Pallet, Call, Config<T>, Storage, Event<T>}, diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 4252eae50d9..b1d6ba6bd9c 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -2709,7 +2709,7 @@ where } fn note_uncle(author: T::AccountId, _age: T::BlockNumber) { Self::reward_by_ids(vec![ - (<pallet_authorship::Module<T>>::author(), 2), + (<pallet_authorship::Pallet<T>>::author(), 2), (author, 1) ]) } diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 188eda80109..c8556a806a4 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -96,6 +96,7 @@ frame_support::construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event<T>}, + Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, Balances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>}, Staking: staking::{Pallet, Call, Config<T>, Storage, Event<T>}, diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 05eb6fdc5e0..634504ccb68 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -2007,7 +2007,7 @@ fn reward_from_authorship_event_handler_works() { ExtBuilder::default().build_and_execute(|| { use pallet_authorship::EventHandler; - assert_eq!(<pallet_authorship::Module<Test>>::author(), 11); + assert_eq!(<pallet_authorship::Pallet<Test>>::author(), 11); <Module<Test>>::note_author(11); <Module<Test>>::note_uncle(21, 1); -- GitLab