From dca583b69c11b0fe21878e607e8b35bca1422d5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 29 Jan 2023 21:56:10 +0100 Subject: [PATCH] Remove `uncles` related code (#13216) The code was added without any clear usage. The inherent for example is not benchmarked and not used. --- Cargo.lock | 24 - Cargo.toml | 2 - bin/node/cli/Cargo.toml | 2 - bin/node/cli/src/service.rs | 12 +- bin/node/runtime/src/lib.rs | 6 - client/consensus/uncles/Cargo.toml | 19 - client/consensus/uncles/README.md | 3 - client/consensus/uncles/src/lib.rs | 45 -- frame/authorship/Cargo.toml | 2 - frame/authorship/src/lib.rs | 602 +----------------- frame/babe/src/mock.rs | 20 +- frame/grandpa/src/mock.rs | 20 +- frame/im-online/src/lib.rs | 4 - frame/im-online/src/mock.rs | 2 - frame/im-online/src/tests.rs | 5 +- frame/staking/src/mock.rs | 18 +- frame/staking/src/pallet/impls.rs | 10 - frame/staking/src/tests.rs | 9 +- .../asset-tx-payment/src/mock.rs | 14 +- primitives/authorship/Cargo.toml | 30 - primitives/authorship/README.md | 3 - primitives/authorship/src/lib.rs | 101 --- 22 files changed, 47 insertions(+), 906 deletions(-) delete mode 100644 client/consensus/uncles/Cargo.toml delete mode 100644 client/consensus/uncles/README.md delete mode 100644 client/consensus/uncles/src/lib.rs delete mode 100644 primitives/authorship/Cargo.toml delete mode 100644 primitives/authorship/README.md delete mode 100644 primitives/authorship/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index c075f065fdd90..29ddf38328519 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4772,7 +4772,6 @@ dependencies = [ "sc-consensus-babe", "sc-consensus-epochs", "sc-consensus-slots", - "sc-consensus-uncles", "sc-executor", "sc-finality-grandpa", "sc-keystore", @@ -4792,7 +4791,6 @@ dependencies = [ "soketto", "sp-api", "sp-authority-discovery", - "sp-authorship", "sp-blockchain", "sp-consensus", "sp-consensus-babe", @@ -5379,7 +5377,6 @@ dependencies = [ "impl-trait-for-tuples", "parity-scale-codec", "scale-info", - "sp-authorship", "sp-core", "sp-io", "sp-runtime", @@ -8299,16 +8296,6 @@ dependencies = [ "substrate-test-runtime-client", ] -[[package]] -name = "sc-consensus-uncles" -version = "0.10.0-dev" -dependencies = [ - "sc-client-api", - "sp-authorship", - "sp-runtime", - "thiserror", -] - [[package]] name = "sc-executor" version = "0.10.0-dev" @@ -9714,17 +9701,6 @@ dependencies = [ "sp-std", ] -[[package]] -name = "sp-authorship" -version = "4.0.0-dev" -dependencies = [ - "async-trait", - "parity-scale-codec", - "sp-inherents", - "sp-runtime", - "sp-std", -] - [[package]] name = "sp-beefy" version = "4.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index bb52305162ef1..b004886e83f9a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,6 @@ members = [ "client/consensus/manual-seal", "client/consensus/pow", "client/consensus/slots", - "client/consensus/uncles", "client/db", "client/executor", "client/executor/common", @@ -177,7 +176,6 @@ members = [ "primitives/arithmetic", "primitives/arithmetic/fuzzer", "primitives/authority-discovery", - "primitives/authorship", "primitives/beefy", "primitives/block-builder", "primitives/blockchain", diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index c529c7c9b8dfd..9ec06d9d9cab1 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -52,7 +52,6 @@ sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" } sp-core = { version = "7.0.0", path = "../../../primitives/core" } sp-runtime = { version = "7.0.0", path = "../../../primitives/runtime" } sp-timestamp = { version = "4.0.0-dev", path = "../../../primitives/timestamp" } -sp-authorship = { version = "4.0.0-dev", path = "../../../primitives/authorship" } sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } sp-keyring = { version = "7.0.0", path = "../../../primitives/keyring" } sp-keystore = { version = "0.13.0", path = "../../../primitives/keystore" } @@ -71,7 +70,6 @@ sc-network = { version = "0.10.0-dev", path = "../../../client/network" } sc-network-common = { version = "0.10.0-dev", path = "../../../client/network/common" } sc-consensus-slots = { version = "0.10.0-dev", path = "../../../client/consensus/slots" } sc-consensus-babe = { version = "0.10.0-dev", path = "../../../client/consensus/babe" } -sc-consensus-uncles = { version = "0.10.0-dev", path = "../../../client/consensus/uncles" } grandpa = { version = "0.10.0-dev", package = "sc-finality-grandpa", path = "../../../client/finality-grandpa" } sc-rpc = { version = "4.0.0-dev", path = "../../../client/rpc" } sc-basic-authorship = { version = "0.10.0-dev", path = "../../../client/basic-authorship" } diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index e329087947c98..b1e9360b4a6c4 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -222,10 +222,7 @@ pub fn new_partial( slot_duration, ); - let uncles = - sp_authorship::InherentDataProvider::<::Header>::check_inherents(); - - Ok((slot, timestamp, uncles)) + Ok((slot, timestamp)) }, &task_manager.spawn_essential_handle(), config.prometheus_registry(), @@ -440,11 +437,6 @@ pub fn new_full_base( create_inherent_data_providers: move |parent, ()| { let client_clone = client_clone.clone(); async move { - let uncles = sc_consensus_uncles::create_uncles_inherent_data_provider( - &*client_clone, - parent, - )?; - let timestamp = sp_timestamp::InherentDataProvider::from_system_time(); let slot = @@ -459,7 +451,7 @@ pub fn new_full_base( &parent, )?; - Ok((slot, timestamp, uncles, storage_proof)) + Ok((slot, timestamp, storage_proof)) } }, force_authoring, diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index bd8a395d15243..30165cdb6f6c7 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -488,14 +488,8 @@ impl pallet_timestamp::Config for Runtime { type WeightInfo = pallet_timestamp::weights::SubstrateWeight; } -parameter_types! { - pub const UncleGenerations: BlockNumber = 5; -} - impl pallet_authorship::Config for Runtime { type FindAuthor = pallet_session::FindAccountFromAuthorIndex; - type UncleGenerations = UncleGenerations; - type FilterUncle = (); type EventHandler = (Staking, ImOnline); } diff --git a/client/consensus/uncles/Cargo.toml b/client/consensus/uncles/Cargo.toml deleted file mode 100644 index 0be48659f9c77..0000000000000 --- a/client/consensus/uncles/Cargo.toml +++ /dev/null @@ -1,19 +0,0 @@ -[package] -name = "sc-consensus-uncles" -version = "0.10.0-dev" -authors = ["Parity Technologies "] -description = "Generic uncle inclusion utilities for consensus" -edition = "2021" -license = "GPL-3.0-or-later WITH Classpath-exception-2.0" -homepage = "https://substrate.io" -repository = "https://github.com/paritytech/substrate/" -readme = "README.md" - -[package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] - -[dependencies] -thiserror = "1.0.30" -sc-client-api = { version = "4.0.0-dev", path = "../../api" } -sp-authorship = { version = "4.0.0-dev", path = "../../../primitives/authorship" } -sp-runtime = { version = "7.0.0", path = "../../../primitives/runtime" } diff --git a/client/consensus/uncles/README.md b/client/consensus/uncles/README.md deleted file mode 100644 index 1b6fed5b9772a..0000000000000 --- a/client/consensus/uncles/README.md +++ /dev/null @@ -1,3 +0,0 @@ -Uncles functionality for Substrate. - -License: GPL-3.0-or-later WITH Classpath-exception-2.0 \ No newline at end of file diff --git a/client/consensus/uncles/src/lib.rs b/client/consensus/uncles/src/lib.rs deleted file mode 100644 index d03c2f8aa6b49..0000000000000 --- a/client/consensus/uncles/src/lib.rs +++ /dev/null @@ -1,45 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0 - -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! Uncles functionality for Substrate. - -use sc_client_api::ProvideUncles; -use sp_runtime::{generic::BlockId, traits::Block as BlockT}; - -#[derive(Debug, thiserror::Error)] -pub enum Error { - #[error("Could not retrieve the block hash for block id: {0:?}")] - NoHashForBlockId(BlockId), -} - -/// Maximum uncles generations we may provide to the runtime. -const MAX_UNCLE_GENERATIONS: u32 = 8; - -/// Create a new [`sp_authorship::InherentDataProvider`] at the given block. -pub fn create_uncles_inherent_data_provider( - client: &C, - parent: B::Hash, -) -> Result, sc_client_api::blockchain::Error> -where - B: BlockT, - C: ProvideUncles, -{ - let uncles = client.uncles(parent, MAX_UNCLE_GENERATIONS.into())?; - - Ok(sp_authorship::InherentDataProvider::new(uncles)) -} diff --git a/frame/authorship/Cargo.toml b/frame/authorship/Cargo.toml index 84d00a6fbc54e..a205c51f9c62d 100644 --- a/frame/authorship/Cargo.toml +++ b/frame/authorship/Cargo.toml @@ -20,7 +20,6 @@ impl-trait-for-tuples = "0.2.2" scale-info = { version = "2.1.1", default-features = false, features = ["derive"] } frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } -sp-authorship = { version = "4.0.0-dev", default-features = false, path = "../../primitives/authorship" } sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" } @@ -35,7 +34,6 @@ std = [ "frame-support/std", "frame-system/std", "scale-info/std", - "sp-authorship/std", "sp-runtime/std", "sp-std/std", ] diff --git a/frame/authorship/src/lib.rs b/frame/authorship/src/lib.rs index 4065472d92368..b5568a6acf77c 100644 --- a/frame/authorship/src/lib.rs +++ b/frame/authorship/src/lib.rs @@ -17,36 +17,12 @@ //! Authorship tracking for FRAME runtimes. //! -//! This tracks the current author of the block and recent uncles. +//! This tracks the current author of the block. #![cfg_attr(not(feature = "std"), no_std)] -use codec::{Decode, Encode, MaxEncodedLen}; -use frame_support::{ - dispatch, - traits::{Defensive, FindAuthor, Get, VerifySeal}, - BoundedSlice, BoundedVec, -}; -use sp_authorship::{InherentError, UnclesInherentData, INHERENT_IDENTIFIER}; -use sp_runtime::traits::{Header as HeaderT, One, Saturating, UniqueSaturatedInto}; -use sp_std::{collections::btree_set::BTreeSet, prelude::*, result}; - -const MAX_UNCLES: usize = 10; - -struct MaxUncleEntryItems(core::marker::PhantomData); -impl Get for MaxUncleEntryItems { - fn get() -> u32 { - // There can be at most `MAX_UNCLES` of `UncleEntryItem::Uncle` and - // one `UncleEntryItem::InclusionHeight` per one `UncleGenerations`, - // so this gives us `MAX_UNCLES + 1` entries per one generation. - // - // There can be one extra generation worth of uncles (e.g. even - // if `UncleGenerations` is zero the pallet will still hold - // one generation worth of uncles). - let max_generations: u32 = T::UncleGenerations::get().unique_saturated_into(); - (MAX_UNCLES as u32 + 1) * (max_generations + 1) - } -} +use frame_support::traits::FindAuthor; +use sp_std::prelude::*; pub use pallet::*; @@ -56,87 +32,8 @@ pub use pallet::*; pub trait EventHandler { /// Note that the given account ID is the author of the current block. fn note_author(author: Author); - - /// Note that the given account ID authored the given uncle, and how many - /// blocks older than the current block it is (age >= 0, so siblings are allowed) - fn note_uncle(author: Author, age: BlockNumber); -} - -/// Additional filtering on uncles that pass preliminary ancestry checks. -/// -/// This should do work such as checking seals -pub trait FilterUncle { - /// An accumulator of data about uncles included. - /// - /// In practice, this is used to validate uncles against others in the same block. - type Accumulator: Default; - - /// Do additional filtering on a seal-checked uncle block, with the accumulated - /// filter. - fn filter_uncle( - header: &Header, - acc: &mut Self::Accumulator, - ) -> Result, &'static str>; } -impl FilterUncle for () { - type Accumulator = (); - fn filter_uncle(_: &H, _acc: &mut Self::Accumulator) -> Result, &'static str> { - Ok(None) - } -} - -/// A filter on uncles which verifies seals and does no additional checks. -/// This is well-suited to consensus modes such as PoW where the cost of -/// equivocating is high. -pub struct SealVerify(sp_std::marker::PhantomData); - -impl> FilterUncle for SealVerify { - type Accumulator = (); - - fn filter_uncle(header: &Header, _acc: &mut ()) -> Result, &'static str> { - T::verify_seal(header) - } -} - -/// A filter on uncles which verifies seals and ensures that there is only -/// one uncle included per author per height. -/// -/// This does O(n log n) work in the number of uncles included. -pub struct OnePerAuthorPerHeight(sp_std::marker::PhantomData<(T, N)>); - -impl FilterUncle for OnePerAuthorPerHeight -where - Header: HeaderT + PartialEq, - Header::Number: Ord, - Author: Clone + PartialEq + Ord, - T: VerifySeal, -{ - type Accumulator = BTreeSet<(Header::Number, Author)>; - - fn filter_uncle( - header: &Header, - acc: &mut Self::Accumulator, - ) -> Result, &'static str> { - let author = T::verify_seal(header)?; - let number = header.number(); - - if let Some(ref author) = author { - if !acc.insert((*number, author.clone())) { - return Err("more than one uncle per number per author included") - } - } - - Ok(author) - } -} - -#[derive(Encode, Decode, sp_runtime::RuntimeDebug, scale_info::TypeInfo, MaxEncodedLen)] -#[cfg_attr(any(feature = "std", test), derive(PartialEq))] -enum UncleEntryItem { - InclusionHeight(BlockNumber), - Uncle(Hash, Option), -} #[frame_support::pallet] pub mod pallet { use super::*; @@ -147,25 +44,6 @@ pub mod pallet { pub trait Config: frame_system::Config { /// Find the author of a block. type FindAuthor: FindAuthor; - /// The number of blocks back we should accept uncles. - /// This means that we will deal with uncle-parents that are - /// `UncleGenerations + 1` before `now`. - #[pallet::constant] - type UncleGenerations: Get; - /// 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; /// An event handler for authored blocks. type EventHandler: EventHandler; } @@ -176,16 +54,7 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { - fn on_initialize(now: T::BlockNumber) -> Weight { - let uncle_generations = T::UncleGenerations::get(); - // prune uncles that are older than the allowed number of generations. - if uncle_generations <= now { - let minimum_height = now - uncle_generations; - Self::prune_old_uncles(minimum_height) - } - - >::put(false); - + fn on_initialize(_: T::BlockNumber) -> Weight { if let Some(author) = Self::author() { T::EventHandler::note_author(author); } @@ -196,125 +65,12 @@ pub mod pallet { fn on_finalize(_: T::BlockNumber) { // ensure we never go to trie with these values. >::kill(); - >::kill(); } } - #[pallet::storage] - /// Uncles - pub(super) type Uncles = StorageValue< - _, - BoundedVec, MaxUncleEntryItems>, - ValueQuery, - >; - #[pallet::storage] /// Author of current block. pub(super) type Author = StorageValue<_, T::AccountId, OptionQuery>; - - #[pallet::storage] - /// Whether uncles were already set in this block. - pub(super) type DidSetUncles = StorageValue<_, bool, ValueQuery>; - - #[pallet::error] - pub enum Error { - /// 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 Pallet { - /// Provide a set of uncles. - #[pallet::call_index(0)] - #[pallet::weight((0, DispatchClass::Mandatory))] - pub fn set_uncles(origin: OriginFor, new_uncles: Vec) -> DispatchResult { - ensure_none(origin)?; - ensure!(new_uncles.len() <= MAX_UNCLES, Error::::TooManyUncles); - - if >::get() { - return Err(Error::::UnclesAlreadySet.into()) - } - >::put(true); - - Self::verify_and_import_uncles(new_uncles) - } - } - - #[pallet::inherent] - impl ProvideInherent for Pallet { - type Call = Call; - type Error = InherentError; - const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER; - - fn create_inherent(data: &InherentData) -> Option { - let uncles = data.uncles().unwrap_or_default(); - let mut new_uncles = Vec::new(); - - if !uncles.is_empty() { - let prev_uncles = >::get(); - let mut existing_hashes: Vec<_> = prev_uncles - .into_iter() - .filter_map(|entry| match entry { - UncleEntryItem::InclusionHeight(_) => None, - UncleEntryItem::Uncle(h, _) => Some(h), - }) - .collect(); - - let mut acc: >::Accumulator = - Default::default(); - - for uncle in uncles { - match Self::verify_uncle(&uncle, &existing_hashes, &mut acc) { - Ok(_) => { - let hash = uncle.hash(); - new_uncles.push(uncle); - existing_hashes.push(hash); - - if new_uncles.len() == MAX_UNCLES { - break - } - }, - Err(_) => { - // skip this uncle - }, - } - } - } - - if new_uncles.is_empty() { - None - } else { - Some(Call::set_uncles { new_uncles }) - } - } - - fn check_inherent( - call: &Self::Call, - _data: &InherentData, - ) -> result::Result<(), Self::Error> { - match call { - Call::set_uncles { ref new_uncles } if new_uncles.len() > MAX_UNCLES => - Err(InherentError::Uncles(Error::::TooManyUncles.as_str().into())), - _ => Ok(()), - } - } - - fn is_inherent(call: &Self::Call) -> bool { - matches!(call, Call::set_uncles { .. }) - } - } } impl Pallet { @@ -323,7 +79,7 @@ impl Pallet { /// This is safe to invoke in `on_initialize` implementations, as well /// as afterwards. pub fn author() -> Option { - // Check the memoized storage value. + // Check the memorized storage value. if let Some(author) = >::get() { return Some(author) } @@ -335,113 +91,22 @@ impl Pallet { a }) } - - fn verify_and_import_uncles(new_uncles: Vec) -> dispatch::DispatchResult { - let now = >::block_number(); - - let mut uncles = >::get(); - uncles - .try_push(UncleEntryItem::InclusionHeight(now)) - .defensive_proof("the list of uncles accepted per generation is bounded, and the number of generations is bounded, so pushing a new element will always succeed") - .map_err(|_| Error::::TooManyUncles)?; - - let mut acc: >::Accumulator = Default::default(); - - for uncle in new_uncles { - let prev_uncles = uncles.iter().filter_map(|entry| match entry { - UncleEntryItem::InclusionHeight(_) => None, - UncleEntryItem::Uncle(h, _) => Some(h), - }); - let maybe_author = Self::verify_uncle(&uncle, prev_uncles, &mut acc)?; - let hash = uncle.hash(); - - if let Some(author) = maybe_author.clone() { - T::EventHandler::note_uncle(author, now - *uncle.number()); - } - uncles.try_push(UncleEntryItem::Uncle(hash, maybe_author)) - .defensive_proof("the list of uncles accepted per generation is bounded, and the number of generations is bounded, so pushing a new element will always succeed") - .map_err(|_| Error::::TooManyUncles)?; - } - - >::put(&uncles); - Ok(()) - } - - fn verify_uncle<'a, I: IntoIterator>( - uncle: &T::Header, - existing_uncles: I, - accumulator: &mut >::Accumulator, - ) -> Result, dispatch::DispatchError> { - let now = >::block_number(); - - let (minimum_height, maximum_height) = { - let uncle_generations = T::UncleGenerations::get(); - let min = now.saturating_sub(uncle_generations); - - (min, now) - }; - - let hash = uncle.hash(); - - if uncle.number() < &One::one() { - return Err(Error::::GenesisUncle.into()) - } - - if uncle.number() > &maximum_height { - return Err(Error::::TooHighUncle.into()) - } - - { - let parent_number = *uncle.number() - One::one(); - let parent_hash = >::block_hash(&parent_number); - if &parent_hash != uncle.parent_hash() { - return Err(Error::::InvalidUncleParent.into()) - } - } - - if uncle.number() < &minimum_height { - return Err(Error::::OldUncle.into()) - } - - let duplicate = existing_uncles.into_iter().any(|h| *h == hash); - let in_chain = >::block_hash(uncle.number()) == hash; - - if duplicate || in_chain { - return Err(Error::::UncleAlreadyIncluded.into()) - } - - // check uncle validity. - T::FilterUncle::filter_uncle(uncle, accumulator).map_err(Into::into) - } - - fn prune_old_uncles(minimum_height: T::BlockNumber) { - let uncles = >::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 pruned_uncles = - >>::try_from(&uncles[prune_index..]) - .expect("after pruning we can't end up with more uncles than we started with"); - - >::put(pruned_uncles); - } } #[cfg(test)] mod tests { use super::*; use crate as pallet_authorship; + use codec::{Decode, Encode}; use frame_support::{ - traits::{ConstU32, ConstU64, OnFinalize, OnInitialize}, + traits::{ConstU32, ConstU64}, ConsensusEngineId, }; use sp_core::H256; use sp_runtime::{ generic::DigestItem, testing::Header, - traits::{BlakeTwo256, IdentityLookup}, + traits::{BlakeTwo256, Header as HeaderT, IdentityLookup}, }; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -454,7 +119,7 @@ mod tests { UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event}, - Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, + Authorship: pallet_authorship::{Pallet, Storage}, } ); @@ -487,8 +152,6 @@ mod tests { impl pallet::Config for Test { type FindAuthor = AuthorGiven; - type UncleGenerations = ConstU64<5>; - type FilterUncle = SealVerify; type EventHandler = (); } @@ -511,34 +174,6 @@ mod tests { } } - pub struct VerifyBlock; - - impl VerifySeal for VerifyBlock { - fn verify_seal(header: &Header) -> Result, &'static str> { - let pre_runtime_digests = header.digest.logs.iter().filter_map(|d| d.as_pre_runtime()); - let seals = header.digest.logs.iter().filter_map(|d| d.as_seal()); - - let author = - AuthorGiven::find_author(pre_runtime_digests).ok_or_else(|| "no author")?; - - for (id, mut seal) in seals { - if id == TEST_ID { - match u64::decode(&mut seal) { - Err(_) => return Err("wrong seal"), - Ok(a) => { - if a != author { - return Err("wrong author in seal") - } - break - }, - } - } - } - - Ok(Some(author)) - } - } - fn seal_header(mut header: Header, author: u64) -> Header { { let digest = header.digest_mut(); @@ -558,196 +193,6 @@ mod tests { t.into() } - #[test] - fn prune_old_uncles_works() { - use UncleEntryItem::*; - new_test_ext().execute_with(|| { - let hash = Default::default(); - let author = Default::default(); - let uncles = vec![ - InclusionHeight(1u64), - Uncle(hash, Some(author)), - Uncle(hash, None), - Uncle(hash, None), - InclusionHeight(2u64), - Uncle(hash, None), - InclusionHeight(3u64), - Uncle(hash, None), - ]; - let uncles = BoundedVec::try_from(uncles).unwrap(); - - ::Uncles::put(uncles); - Authorship::prune_old_uncles(3); - - let uncles = ::Uncles::get(); - assert_eq!(uncles, vec![InclusionHeight(3u64), Uncle(hash, None)]); - }) - } - - #[test] - fn rejects_bad_uncles() { - new_test_ext().execute_with(|| { - let author_a = 69; - - struct CanonChain { - inner: Vec
, - } - - impl CanonChain { - fn best_hash(&self) -> H256 { - self.inner.last().unwrap().hash() - } - - fn canon_hash(&self, index: usize) -> H256 { - self.inner[index].hash() - } - - fn header(&self, index: usize) -> &Header { - &self.inner[index] - } - - fn push(&mut self, header: Header) { - self.inner.push(header) - } - } - - let mut canon_chain = CanonChain { - inner: vec![seal_header( - create_header(0, Default::default(), Default::default()), - 999, - )], - }; - - let initialize_block = |number, hash: H256| { - System::reset_events(); - System::initialize(&number, &hash, &Default::default()) - }; - - for number in 1..8 { - initialize_block(number, canon_chain.best_hash()); - let header = seal_header(System::finalize(), author_a); - canon_chain.push(header); - } - - // initialize so system context is set up correctly. - initialize_block(8, canon_chain.best_hash()); - - // 2 of the same uncle at once - { - let uncle_a = seal_header( - create_header(3, canon_chain.canon_hash(2), [1; 32].into()), - author_a, - ); - assert_eq!( - Authorship::verify_and_import_uncles(vec![uncle_a.clone(), uncle_a.clone()]), - Err(Error::::UncleAlreadyIncluded.into()), - ); - } - - // 2 of the same uncle at different times. - { - let uncle_a = seal_header( - create_header(3, canon_chain.canon_hash(2), [1; 32].into()), - author_a, - ); - - assert!(Authorship::verify_and_import_uncles(vec![uncle_a.clone()]).is_ok()); - - assert_eq!( - Authorship::verify_and_import_uncles(vec![uncle_a.clone()]), - Err(Error::::UncleAlreadyIncluded.into()), - ); - } - - // same uncle as ancestor. - { - let uncle_clone = canon_chain.header(5).clone(); - - assert_eq!( - Authorship::verify_and_import_uncles(vec![uncle_clone]), - Err(Error::::UncleAlreadyIncluded.into()), - ); - } - - // uncle without valid seal. - { - let unsealed = create_header(3, canon_chain.canon_hash(2), [2; 32].into()); - assert_eq!( - Authorship::verify_and_import_uncles(vec![unsealed]), - Err("no author".into()), - ); - } - - // old uncles can't get in. - { - assert_eq!(System::block_number(), 8); - - let gen_2 = seal_header( - create_header(2, canon_chain.canon_hash(1), [3; 32].into()), - author_a, - ); - - assert_eq!( - Authorship::verify_and_import_uncles(vec![gen_2]), - Err(Error::::OldUncle.into()), - ); - } - - // siblings are also allowed - { - let other_8 = seal_header( - create_header(8, canon_chain.canon_hash(7), [1; 32].into()), - author_a, - ); - - assert!(Authorship::verify_and_import_uncles(vec![other_8]).is_ok()); - } - }); - } - - #[test] - fn maximum_bound() { - new_test_ext().execute_with(|| { - let mut max_item_count = 0; - - let mut author_counter = 0; - let mut current_depth = 1; - let mut parent_hash: H256 = [1; 32].into(); - let mut uncles = vec![]; - - // We deliberately run this for more generations than the limit - // so that we can get the `Uncles` to hit its cap. - for _ in 0..<::UncleGenerations as Get>::get() + 3 { - let new_uncles: Vec<_> = (0..MAX_UNCLES) - .map(|_| { - System::reset_events(); - System::initialize(¤t_depth, &parent_hash, &Default::default()); - // Increment the author on every block to make sure the hash's always - // different. - author_counter += 1; - seal_header(System::finalize(), author_counter) - }) - .collect(); - - author_counter += 1; - System::reset_events(); - System::initialize(¤t_depth, &parent_hash, &Default::default()); - Authorship::on_initialize(current_depth); - Authorship::set_uncles(RuntimeOrigin::none(), uncles).unwrap(); - Authorship::on_finalize(current_depth); - max_item_count = - std::cmp::max(max_item_count, ::Uncles::get().len()); - - let new_parent = seal_header(System::finalize(), author_counter); - parent_hash = new_parent.hash(); - uncles = new_uncles; - current_depth += 1; - } - - assert_eq!(max_item_count, MaxUncleEntryItems::::get() as usize); - }); - } - #[test] fn sets_author_lazily() { new_test_ext().execute_with(|| { @@ -762,33 +207,4 @@ mod tests { assert_eq!(Authorship::author(), Some(author)); }); } - - #[test] - fn one_uncle_per_author_per_number() { - type Filter = OnePerAuthorPerHeight; - - let author_a = 42; - let author_b = 43; - - let mut acc: >::Accumulator = Default::default(); - let header_a1 = seal_header(create_header(1, Default::default(), [1; 32].into()), author_a); - let header_b1 = seal_header(create_header(1, Default::default(), [1; 32].into()), author_b); - - let header_a2_1 = - seal_header(create_header(2, Default::default(), [1; 32].into()), author_a); - let header_a2_2 = - seal_header(create_header(2, Default::default(), [2; 32].into()), author_a); - - let mut check_filter = move |uncle| Filter::filter_uncle(uncle, &mut acc); - - // same height, different author is OK. - assert_eq!(check_filter(&header_a1), Ok(Some(author_a))); - assert_eq!(check_filter(&header_b1), Ok(Some(author_b))); - - // same author, different height. - assert_eq!(check_filter(&header_a2_1), Ok(Some(author_a))); - - // same author, same height (author a, height 2) - assert!(check_filter(&header_a2_2).is_err()); - } } diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index abae1b7e33157..b09dc29f6d60a 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -52,15 +52,15 @@ frame_support::construct_runtime!( NodeBlock = Block, UncheckedExtrinsic = UncheckedExtrinsic, { - System: frame_system::{Pallet, Call, Config, Storage, Event}, - Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, - Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - Historical: pallet_session_historical::{Pallet}, - Offences: pallet_offences::{Pallet, Storage, Event}, - Babe: pallet_babe::{Pallet, Call, Storage, Config, ValidateUnsigned}, - Staking: pallet_staking::{Pallet, Call, Storage, Config, Event}, - Session: pallet_session::{Pallet, Call, Storage, Event, Config}, - Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, + System: frame_system, + Authorship: pallet_authorship, + Balances: pallet_balances, + Historical: pallet_session_historical, + Offences: pallet_offences, + Babe: pallet_babe, + Staking: pallet_staking, + Session: pallet_session, + Timestamp: pallet_timestamp, } ); @@ -124,8 +124,6 @@ impl pallet_session::historical::Config for Test { impl pallet_authorship::Config for Test { type FindAuthor = pallet_session::FindAccountFromAuthorIndex; - type UncleGenerations = ConstU64<0>; - type FilterUncle = (); type EventHandler = (); } diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index c618705af0651..54f34008abc56 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -51,15 +51,15 @@ frame_support::construct_runtime!( NodeBlock = Block, UncheckedExtrinsic = UncheckedExtrinsic, { - System: frame_system::{Pallet, Call, Config, Storage, Event}, - Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, - Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, - Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - Staking: pallet_staking::{Pallet, Call, Config, Storage, Event}, - Session: pallet_session::{Pallet, Call, Storage, Event, Config}, - Grandpa: pallet_grandpa::{Pallet, Call, Storage, Config, Event, ValidateUnsigned}, - Offences: pallet_offences::{Pallet, Storage, Event}, - Historical: pallet_session_historical::{Pallet}, + System: frame_system, + Authorship: pallet_authorship, + Timestamp: pallet_timestamp, + Balances: pallet_balances, + Staking: pallet_staking, + Session: pallet_session, + Grandpa: pallet_grandpa, + Offences: pallet_offences, + Historical: pallet_session_historical, } ); @@ -129,8 +129,6 @@ impl pallet_session::historical::Config for Test { impl pallet_authorship::Config for Test { type FindAuthor = (); - type UncleGenerations = ConstU64<0>; - type FilterUncle = (); type EventHandler = (); } diff --git a/frame/im-online/src/lib.rs b/frame/im-online/src/lib.rs index f23e610a4874d..8c1f46978e8bc 100644 --- a/frame/im-online/src/lib.rs +++ b/frame/im-online/src/lib.rs @@ -607,10 +607,6 @@ impl fn note_author(author: ValidatorId) { Self::note_authorship(author); } - - fn note_uncle(author: ValidatorId, _age: T::BlockNumber) { - Self::note_authorship(author); - } } impl Pallet { diff --git a/frame/im-online/src/mock.rs b/frame/im-online/src/mock.rs index bcb13fa1be837..783e68dfede9f 100644 --- a/frame/im-online/src/mock.rs +++ b/frame/im-online/src/mock.rs @@ -169,8 +169,6 @@ impl pallet_session::historical::Config for Runtime { impl pallet_authorship::Config for Runtime { type FindAuthor = (); - type UncleGenerations = ConstU64<5>; - type FilterUncle = (); type EventHandler = ImOnline; } diff --git a/frame/im-online/src/tests.rs b/frame/im-online/src/tests.rs index 366119278d836..2c026f7176b65 100644 --- a/frame/im-online/src/tests.rs +++ b/frame/im-online/src/tests.rs @@ -308,11 +308,10 @@ fn should_mark_online_validator_when_block_is_authored() { // when ImOnline::note_author(1); - ImOnline::note_uncle(2, 0); // then assert!(ImOnline::is_online(0)); - assert!(ImOnline::is_online(1)); + assert!(!ImOnline::is_online(1)); assert!(!ImOnline::is_online(2)); }); } @@ -338,7 +337,7 @@ fn should_not_send_a_report_if_already_online() { assert_eq!(Session::current_index(), 2); assert_eq!(Session::validators(), vec![1, 2, 3]); ImOnline::note_author(2); - ImOnline::note_uncle(3, 0); + ImOnline::note_author(3); // when UintAuthorityId::set_all_keys(vec![1, 2, 3]); diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index edf8d29645a55..81b48694fae99 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -91,14 +91,14 @@ frame_support::construct_runtime!( NodeBlock = Block, UncheckedExtrinsic = UncheckedExtrinsic, { - System: frame_system::{Pallet, Call, Config, Storage, Event}, - Authorship: pallet_authorship::{Pallet, Call, Storage, Inherent}, - Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, - Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - Staking: pallet_staking::{Pallet, Call, Config, Storage, Event}, - Session: pallet_session::{Pallet, Call, Storage, Event, Config}, - Historical: pallet_session::historical::{Pallet, Storage}, - VoterBagsList: pallet_bags_list::::{Pallet, Call, Storage, Event}, + System: frame_system, + Authorship: pallet_authorship, + Timestamp: pallet_timestamp, + Balances: pallet_balances, + Staking: pallet_staking, + Session: pallet_session, + Historical: pallet_session::historical, + VoterBagsList: pallet_bags_list::, } ); @@ -182,8 +182,6 @@ impl pallet_session::historical::Config for Test { } impl pallet_authorship::Config for Test { type FindAuthor = Author11; - type UncleGenerations = ConstU64<0>; - type FilterUncle = (); type EventHandler = Pallet; } diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index acb995ed24320..faf807eee8275 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1222,8 +1222,6 @@ impl historical::SessionManager pallet_authorship::EventHandler for Pallet where T: Config + pallet_authorship::Config + pallet_session::Config, @@ -1231,14 +1229,6 @@ where fn note_author(author: T::AccountId) { Self::reward_by_ids(vec![(author, 20)]) } - fn note_uncle(uncle_author: T::AccountId, _age: T::BlockNumber) { - // defensive-only: block author must exist. - if let Some(block_author) = >::author() { - Self::reward_by_ids(vec![(block_author, 2), (uncle_author, 1)]) - } else { - crate::log!(warn, "block author not set, this should never happen"); - } - } } /// This is intended to be used with `FilterHistoricalOffences`. diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 9c017a53c145b..307c3996a82b6 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -2250,9 +2250,7 @@ fn reward_from_authorship_event_handler_works() { assert_eq!(>::author(), Some(11)); Pallet::::note_author(11); - Pallet::::note_uncle(21, 1); - // Rewarding the same two times works. - Pallet::::note_uncle(11, 1); + Pallet::::note_author(11); // Not mandatory but must be coherent with rewards assert_eq_uvec!(Session::validators(), vec![11, 21]); @@ -2261,10 +2259,7 @@ fn reward_from_authorship_event_handler_works() { // 11 is rewarded as a block producer and uncle referencer and uncle producer assert_eq!( ErasRewardPoints::::get(active_era()), - EraRewardPoints { - individual: vec![(11, 20 + 2 * 2 + 1), (21, 1)].into_iter().collect(), - total: 26, - }, + EraRewardPoints { individual: vec![(11, 20 * 2)].into_iter().collect(), total: 40 }, ); }) } diff --git a/frame/transaction-payment/asset-tx-payment/src/mock.rs b/frame/transaction-payment/asset-tx-payment/src/mock.rs index ddb02f5a611ff..60fb2e6c5e03e 100644 --- a/frame/transaction-payment/asset-tx-payment/src/mock.rs +++ b/frame/transaction-payment/asset-tx-payment/src/mock.rs @@ -45,12 +45,12 @@ frame_support::construct_runtime!( NodeBlock = Block, UncheckedExtrinsic = UncheckedExtrinsic, { - System: system::{Pallet, Call, Config, Storage, Event}, - Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, - TransactionPayment: pallet_transaction_payment::{Pallet, Storage, Event}, - Assets: pallet_assets::{Pallet, Call, Storage, Event}, - Authorship: pallet_authorship::{Pallet, Call, Storage}, - AssetTxPayment: pallet_asset_tx_payment::{Pallet, Event}, + System: system, + Balances: pallet_balances, + TransactionPayment: pallet_transaction_payment, + Assets: pallet_assets, + Authorship: pallet_authorship, + AssetTxPayment: pallet_asset_tx_payment, } ); @@ -187,8 +187,6 @@ impl FindAuthor for HardcodedAuthor { impl pallet_authorship::Config for Runtime { type FindAuthor = HardcodedAuthor; - type UncleGenerations = (); - type FilterUncle = (); type EventHandler = (); } diff --git a/primitives/authorship/Cargo.toml b/primitives/authorship/Cargo.toml deleted file mode 100644 index 31ea3b2d4c8f3..0000000000000 --- a/primitives/authorship/Cargo.toml +++ /dev/null @@ -1,30 +0,0 @@ -[package] -name = "sp-authorship" -version = "4.0.0-dev" -authors = ["Parity Technologies "] -description = "Authorship primitives" -edition = "2021" -license = "Apache-2.0" -homepage = "https://substrate.io" -repository = "https://github.com/paritytech/substrate/" -readme = "README.md" - -[package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] - -[dependencies] -async-trait = { version = "0.1.57", optional = true } -codec = { package = "parity-scale-codec", version = "3.2.2", default-features = false, features = ["derive"] } -sp-inherents = { version = "4.0.0-dev", default-features = false, path = "../inherents" } -sp-runtime = { version = "7.0.0", default-features = false, path = "../runtime" } -sp-std = { version = "5.0.0", default-features = false, path = "../std" } - -[features] -default = [ "std" ] -std = [ - "async-trait", - "codec/std", - "sp-inherents/std", - "sp-runtime/std", - "sp-std/std", -] diff --git a/primitives/authorship/README.md b/primitives/authorship/README.md deleted file mode 100644 index 1aa1805cfc5e7..0000000000000 --- a/primitives/authorship/README.md +++ /dev/null @@ -1,3 +0,0 @@ -Authorship Primitives - -License: Apache-2.0 \ No newline at end of file diff --git a/primitives/authorship/src/lib.rs b/primitives/authorship/src/lib.rs deleted file mode 100644 index 6ff6607a896cc..0000000000000 --- a/primitives/authorship/src/lib.rs +++ /dev/null @@ -1,101 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Authorship Primitives - -#![cfg_attr(not(feature = "std"), no_std)] - -use sp_std::{prelude::*, result::Result}; - -#[cfg(feature = "std")] -use codec::Decode; -use codec::Encode; -use sp_inherents::{Error, InherentData, InherentIdentifier, IsFatalError}; -use sp_runtime::{traits::Header as HeaderT, RuntimeString}; - -/// The identifier for the `uncles` inherent. -pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"uncles00"; - -/// Errors that can occur while checking the authorship inherent. -#[derive(Encode, sp_runtime::RuntimeDebug)] -#[cfg_attr(feature = "std", derive(Decode))] -pub enum InherentError { - Uncles(RuntimeString), -} - -impl IsFatalError for InherentError { - fn is_fatal_error(&self) -> bool { - match self { - InherentError::Uncles(_) => true, - } - } -} - -/// Auxiliary trait to extract uncles inherent data. -pub trait UnclesInherentData { - /// Get uncles. - fn uncles(&self) -> Result, Error>; -} - -impl UnclesInherentData for InherentData { - fn uncles(&self) -> Result, Error> { - Ok(self.get_data(&INHERENT_IDENTIFIER)?.unwrap_or_default()) - } -} - -/// Provider for inherent data. -#[cfg(feature = "std")] -pub struct InherentDataProvider { - uncles: Vec, -} - -#[cfg(feature = "std")] -impl InherentDataProvider { - /// Create a new inherent data provider with the given `uncles`. - pub fn new(uncles: Vec) -> Self { - InherentDataProvider { uncles } - } - - /// Create a new instance that is usable for checking inherents. - /// - /// This will always return an empty vec of uncles. - pub fn check_inherents() -> Self { - Self { uncles: Vec::new() } - } -} - -#[cfg(feature = "std")] -#[async_trait::async_trait] -impl sp_inherents::InherentDataProvider for InherentDataProvider { - async fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> { - inherent_data.put_data(INHERENT_IDENTIFIER, &self.uncles) - } - - async fn try_handle_error( - &self, - identifier: &InherentIdentifier, - mut error: &[u8], - ) -> Option> { - if *identifier != INHERENT_IDENTIFIER { - return None - } - - let error = InherentError::decode(&mut error).ok()?; - - Some(Err(Error::Application(Box::from(format!("{:?}", error))))) - } -}