diff --git a/frame/authorship/src/lib.rs b/frame/authorship/src/lib.rs index 561db20849c2f..8ddccfd9cf939 100644 --- a/frame/authorship/src/lib.rs +++ b/frame/authorship/src/lib.rs @@ -21,17 +21,33 @@ #![cfg_attr(not(feature = "std"), no_std)] -use codec::{Decode, Encode}; +use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ dispatch, - traits::{FindAuthor, Get, VerifySeal}, + traits::{Defensive, FindAuthor, Get, VerifySeal}, + BoundedSlice, BoundedVec, }; use sp_authorship::{InherentError, UnclesInherentData, INHERENT_IDENTIFIER}; -use sp_runtime::traits::{Header as HeaderT, One, Saturating}; +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) + } +} + pub use pallet::*; /// An event handler for the authorship pallet. There is a dummy implementation @@ -115,7 +131,7 @@ where } } -#[derive(Encode, Decode, sp_runtime::RuntimeDebug, scale_info::TypeInfo)] +#[derive(Encode, Decode, sp_runtime::RuntimeDebug, scale_info::TypeInfo, MaxEncodedLen)] #[cfg_attr(any(feature = "std", test), derive(PartialEq))] enum UncleEntryItem { InclusionHeight(BlockNumber), @@ -156,7 +172,6 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] - #[pallet::without_storage_info] pub struct Pallet(_); #[pallet::hooks] @@ -187,8 +202,11 @@ pub mod pallet { #[pallet::storage] /// Uncles - pub(super) type Uncles = - StorageValue<_, Vec>, ValueQuery>; + pub(super) type Uncles = StorageValue< + _, + BoundedVec, MaxUncleEntryItems>, + ValueQuery, + >; #[pallet::storage] /// Author of current block. @@ -321,7 +339,10 @@ impl Pallet { let now = >::block_number(); let mut uncles = >::get(); - uncles.push(UncleEntryItem::InclusionHeight(now)); + 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(); @@ -336,7 +357,9 @@ impl Pallet { if let Some(author) = maybe_author.clone() { T::EventHandler::note_uncle(author, now - *uncle.number()); } - uncles.push(UncleEntryItem::Uncle(hash, maybe_author)); + 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); @@ -397,8 +420,11 @@ impl Pallet { 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(&uncles[prune_index..]); + >::put(pruned_uncles); } } @@ -408,7 +434,7 @@ mod tests { use crate as pallet_authorship; use frame_support::{ parameter_types, - traits::{ConstU32, ConstU64}, + traits::{ConstU32, ConstU64, OnFinalize, OnInitialize}, ConsensusEngineId, }; use sp_core::H256; @@ -553,6 +579,7 @@ mod tests { InclusionHeight(3u64), Uncle(hash, None), ]; + let uncles = BoundedVec::try_from(uncles).unwrap(); ::Uncles::put(uncles); Authorship::prune_old_uncles(3); @@ -683,6 +710,49 @@ mod tests { }); } + #[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(Origin::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(|| {