Skip to content
This repository has been archived by the owner on Feb 21, 2024. It is now read-only.

Commit

Permalink
Upgrade authorship pallet to Frame-v2 (paritytech#8663)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people authored and nazar-pc committed Aug 8, 2021
1 parent 35adafe commit 5d1adbd
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 81 deletions.
171 changes: 94 additions & 77 deletions frame/authorship/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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();
Expand All @@ -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()
Expand All @@ -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();
Expand All @@ -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(())
}

Expand Down Expand Up @@ -325,19 +342,19 @@ 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,
});
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;
Expand All @@ -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,
Expand Down Expand Up @@ -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>;
Expand Down
2 changes: 1 addition & 1 deletion frame/babe/src/equivocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ where
}

fn block_author() -> Option<T::AccountId> {
Some(<pallet_authorship::Module<T>>::author())
Some(<pallet_authorship::Pallet<T>>::author())
}
}

Expand Down
1 change: 1 addition & 0 deletions frame/babe/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
2 changes: 1 addition & 1 deletion frame/grandpa/src/equivocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ where
}

fn block_author() -> Option<T::AccountId> {
Some(<pallet_authorship::Module<T>>::author())
Some(<pallet_authorship::Pallet<T>>::author())
}
}

Expand Down
1 change: 1 addition & 0 deletions frame/grandpa/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>},
Expand Down
2 changes: 1 addition & 1 deletion frame/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
])
}
Expand Down
1 change: 1 addition & 0 deletions frame/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>},
Expand Down
2 changes: 1 addition & 1 deletion frame/staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 5d1adbd

Please sign in to comment.