From 849a6c35fd21623de6dd7a65c2409ee4fc261abc Mon Sep 17 00:00:00 2001
From: Koute <koute@users.noreply.github.com>
Date: Thu, 16 Jun 2022 20:48:05 +0900
Subject: [PATCH] Remove `without_storage_info` for the authorship pallet
(#11610)
* Remove `without_storage_info` for the authorship pallet
* Tweak impl bounds style
* Use `defensive_proof` instead of `expect`
---
substrate/frame/authorship/src/lib.rs | 92 +++++++++++++++++++++++----
1 file changed, 81 insertions(+), 11 deletions(-)
diff --git a/substrate/frame/authorship/src/lib.rs b/substrate/frame/authorship/src/lib.rs
index 561db20849c..8ddccfd9cf9 100644
--- a/substrate/frame/authorship/src/lib.rs
+++ b/substrate/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<T>(core::marker::PhantomData<T>);
+impl<T: Config> Get<u32> for MaxUncleEntryItems<T> {
+ 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<BlockNumber, Hash, Author> {
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<T>(_);
#[pallet::hooks]
@@ -187,8 +202,11 @@ pub mod pallet {
#[pallet::storage]
/// Uncles
- pub(super) type Uncles<T: Config> =
- StorageValue<_, Vec<UncleEntryItem<T::BlockNumber, T::Hash, T::AccountId>>, ValueQuery>;
+ pub(super) type Uncles<T: Config> = StorageValue<
+ _,
+ BoundedVec<UncleEntryItem<T::BlockNumber, T::Hash, T::AccountId>, MaxUncleEntryItems<T>>,
+ ValueQuery,
+ >;
#[pallet::storage]
/// Author of current block.
@@ -321,7 +339,10 @@ impl<T: Config> Pallet<T> {
let now = <frame_system::Pallet<T>>::block_number();
let mut uncles = <Uncles<T>>::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::<T>::TooManyUncles)?;
let mut acc: <T::FilterUncle as FilterUncle<_, _>>::Accumulator = Default::default();
@@ -336,7 +357,9 @@ impl<T: Config> Pallet<T> {
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::<T>::TooManyUncles)?;
}
<Uncles<T>>::put(&uncles);
@@ -397,8 +420,11 @@ impl<T: Config> Pallet<T> {
UncleEntryItem::InclusionHeight(height) => height < &minimum_height,
});
let prune_index = prune_entries.count();
+ let pruned_uncles =
+ <BoundedSlice<'_, _, MaxUncleEntryItems<T>>>::try_from(&uncles[prune_index..])
+ .expect("after pruning we can't end up with more uncles than we started with");
- <Uncles<T>>::put(&uncles[prune_index..]);
+ <Uncles<T>>::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();
<Authorship as Store>::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..<<Test as Config>::UncleGenerations as Get<u64>>::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, <Authorship as Store>::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::<Test>::get() as usize);
+ });
+ }
+
#[test]
fn sets_author_lazily() {
new_test_ext().execute_with(|| {
--
GitLab