Bound staking storage items (paritytech#12230)

* replace pallet level unboundedness to individual storage items

* bound structs

* bounding history depth

* defensive error

* use the era history depth from config

* clean up history depth from storage in v11

* keep the name HistoryDepth for the new configuration value

* use u32 for history depth in node runtime

* improve doc comments

* add HistoryDepth to mock runtimes with pallet-staking

* rustfmt

* refactor and doc improve

* apply re-benchmarked weight for staking

* pr feedback improvements

* test for claimed rewards following the expected bounds

* refactor test to calculate first and last reward era programmatically

* verify previous eras cannot be claimed

* add migration v12

* ".git/.scripts/bench-bot.sh" pallet dev pallet_staking

* fix compiler error

* corrupting history depth does not lead to catastrophic issue

* fix new line

* remove unused import

* fmt

* add test to document scenario where history depth is reduced without migration

* fmt

* Update frame/staking/src/lib.rs

Co-authored-by: Kian Paimani <[email protected]>

* Update frame/staking/src/migrations.rs

Co-authored-by: Kian Paimani <[email protected]>

* doc for all storage items bounded by HistoryDepth

* Update frame/staking/src/pallet/mod.rs

Co-authored-by: Kian Paimani <[email protected]>

* Update frame/staking/src/tests.rs

Co-authored-by: Kian Paimani <[email protected]>

* pr feedback fixes

* Update frame/staking/src/tests.rs

Co-authored-by: Kian Paimani <[email protected]>

* remove extra checks

* fix merge

* fmt

Co-authored-by: command-bot <>
Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: kianenigma <[email protected]>

Author: 2 people

bin/node/runtime/src/lib.rs

@@ -535,6 +535,7 @@ parameter_types! {
 	pub const MaxNominatorRewardedPerValidator: u32 = 256;
 	pub const OffendingValidatorsThreshold: Perbill = Perbill::from_percent(17);
 	pub OffchainRepeat: BlockNumber = 5;
+	pub HistoryDepth: u32 = 84;
 }
 
 pub struct StakingBenchmarkingConfig;
@@ -572,6 +573,7 @@ impl pallet_staking::Config for Runtime {
 	// This a placeholder, to be introduced in the next PR as an instance of bags-list
 	type TargetList = pallet_staking::UseValidatorsMap<Self>;
 	type MaxUnlockingChunks = ConstU32<32>;
+	type HistoryDepth = HistoryDepth;
 	type OnStakerSlash = NominationPools;
 	type WeightInfo = pallet_staking::weights::SubstrateWeight<Runtime>;
 	type BenchmarkingConfig = StakingBenchmarkingConfig;

frame/babe/src/mock.rs

@@ -204,6 +204,7 @@ impl pallet_staking::Config for Test {
 	type VoterList = pallet_staking::UseNominatorsAndValidatorsMap<Self>;
 	type TargetList = pallet_staking::UseValidatorsMap<Self>;
 	type MaxUnlockingChunks = ConstU32<32>;
+	type HistoryDepth = ConstU32<84>;
 	type OnStakerSlash = ();
 	type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig;
 	type WeightInfo = ();

frame/grandpa/src/mock.rs

@@ -208,6 +208,7 @@ impl pallet_staking::Config for Test {
 	type VoterList = pallet_staking::UseNominatorsAndValidatorsMap<Self>;
 	type TargetList = pallet_staking::UseValidatorsMap<Self>;
 	type MaxUnlockingChunks = ConstU32<32>;
+	type HistoryDepth = ConstU32<84>;
 	type OnStakerSlash = ();
 	type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig;
 	type WeightInfo = ();

frame/nomination-pools/benchmarking/src/mock.rs

@@ -115,6 +115,7 @@ impl pallet_staking::Config for Runtime {
 	type VoterList = VoterList;
 	type TargetList = pallet_staking::UseValidatorsMap<Self>;
 	type MaxUnlockingChunks = ConstU32<32>;
+	type HistoryDepth = ConstU32<84>;
 	type OnStakerSlash = Pools;
 	type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig;
 	type WeightInfo = ();

frame/nomination-pools/test-staking/src/mock.rs

@@ -129,6 +129,7 @@ impl pallet_staking::Config for Runtime {
 	type VoterList = VoterList;
 	type TargetList = pallet_staking::UseValidatorsMap<Self>;
 	type MaxUnlockingChunks = ConstU32<32>;
+	type HistoryDepth = ConstU32<84>;
 	type OnStakerSlash = Pools;
 	type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig;
 	type WeightInfo = ();

frame/offences/benchmarking/src/mock.rs

@@ -182,6 +182,7 @@ impl pallet_staking::Config for Test {
 	type VoterList = pallet_staking::UseNominatorsAndValidatorsMap<Self>;
 	type TargetList = pallet_staking::UseValidatorsMap<Self>;
 	type MaxUnlockingChunks = ConstU32<32>;
+	type HistoryDepth = ConstU32<84>;
 	type OnStakerSlash = ();
 	type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig;
 	type WeightInfo = ();

frame/session/benchmarking/src/mock.rs

@@ -174,6 +174,7 @@ impl pallet_staking::Config for Test {
 	type ElectionProvider = onchain::UnboundedExecution<OnChainSeqPhragmen>;
 	type GenesisElectionProvider = Self::ElectionProvider;
 	type MaxUnlockingChunks = ConstU32<32>;
+	type HistoryDepth = ConstU32<84>;
 	type VoterList = pallet_staking::UseNominatorsAndValidatorsMap<Self>;
 	type TargetList = pallet_staking::UseValidatorsMap<Self>;
 	type OnStakerSlash = ();

frame/staking/src/benchmarking.rs

@@ -660,25 +660,6 @@ benchmarks! {
 		assert!(original_bonded < new_bonded);
 	}
 
-	set_history_depth {
-		let e in 1 .. 100;
-		HistoryDepth::<T>::put(e);
-		CurrentEra::<T>::put(e);
-		let dummy = || -> T::AccountId { codec::Decode::decode(&mut TrailingZeroInput::zeroes()).unwrap() };
-		for i in 0 .. e {
-			<ErasStakers<T>>::insert(i, dummy(), Exposure::<T::AccountId, BalanceOf<T>>::default());
-			<ErasStakersClipped<T>>::insert(i, dummy(), Exposure::<T::AccountId, BalanceOf<T>>::default());
-			<ErasValidatorPrefs<T>>::insert(i, dummy(), ValidatorPrefs::default());
-			<ErasValidatorReward<T>>::insert(i, BalanceOf::<T>::one());
-			<ErasRewardPoints<T>>::insert(i, EraRewardPoints::<T::AccountId>::default());
-			<ErasTotalStake<T>>::insert(i, BalanceOf::<T>::one());
-			ErasStartSessionIndex::<T>::insert(i, i);
-		}
-	}: _(RawOrigin::Root, EraIndex::zero(), u32::MAX)
-	verify {
-		assert_eq!(HistoryDepth::<T>::get(), 0);
-	}
-
 	reap_stash {
 		let s in 1 .. MAX_SPANS;
 		// clean up any existing state.
@@ -698,7 +679,7 @@ benchmarks! {
 			active: T::Currency::minimum_balance() - One::one(),
 			total: T::Currency::minimum_balance() - One::one(),
 			unlocking: Default::default(),
-			claimed_rewards: vec![],
+			claimed_rewards: Default::default(),
 		};
 		Ledger::<T>::insert(&controller, l);
 

frame/staking/src/lib.rs

@@ -299,12 +299,12 @@ pub mod weights;
 
 mod pallet;
 
-use codec::{Decode, Encode, HasCompact};
+use codec::{Decode, Encode, HasCompact, MaxEncodedLen};
 use frame_support::{
 	parameter_types,
 	traits::{Currency, Defensive, Get},
 	weights::Weight,
-	BoundedVec, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound,
+	BoundedVec, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound,
 };
 use scale_info::TypeInfo;
 use sp_runtime::{
@@ -354,7 +354,7 @@ parameter_types! {
 }
 
 /// Information regarding the active era (era in used in session).
-#[derive(Encode, Decode, RuntimeDebug, TypeInfo)]
+#[derive(Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)]
 pub struct ActiveEraInfo {
 	/// Index of era.
 	pub index: EraIndex,
@@ -395,7 +395,7 @@ pub enum StakerStatus<AccountId> {
 }
 
 /// A destination account for payment.
-#[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
+#[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)]
 pub enum RewardDestination<AccountId> {
 	/// Pay into the stash account, increasing the amount at stake accordingly.
 	Staked,
@@ -416,7 +416,7 @@ impl<AccountId> Default for RewardDestination<AccountId> {
 }
 
 /// Preference of what happens regarding validation.
-#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo, Default)]
+#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo, Default, MaxEncodedLen)]
 pub struct ValidatorPrefs {
 	/// Reward that validator takes up-front; only the rest is split between themselves and
 	/// nominators.
@@ -429,8 +429,8 @@ pub struct ValidatorPrefs {
 }
 
 /// Just a Balance/BlockNumber tuple to encode when a chunk of funds will be unlocked.
-#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
-pub struct UnlockChunk<Balance: HasCompact> {
+#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)]
+pub struct UnlockChunk<Balance: HasCompact + MaxEncodedLen> {
 	/// Amount of funds to be unlocked.
 	#[codec(compact)]
 	value: Balance,
@@ -440,7 +440,16 @@ pub struct UnlockChunk<Balance: HasCompact> {
 }
 
 /// The ledger of a (bonded) stash.
-#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebugNoBound, TypeInfo)]
+#[derive(
+	PartialEqNoBound,
+	EqNoBound,
+	CloneNoBound,
+	Encode,
+	Decode,
+	RuntimeDebugNoBound,
+	TypeInfo,
+	MaxEncodedLen,
+)]
 #[scale_info(skip_type_params(T))]
 pub struct StakingLedger<T: Config> {
 	/// The stash account whose balance is actually locked and at stake.
@@ -459,7 +468,7 @@ pub struct StakingLedger<T: Config> {
 	pub unlocking: BoundedVec<UnlockChunk<BalanceOf<T>>, MaxUnlockingChunks>,
 	/// List of eras for which the stakers behind a validator have claimed rewards. Only updated
 	/// for validators.
-	pub claimed_rewards: Vec<EraIndex>,
+	pub claimed_rewards: BoundedVec<EraIndex, T::HistoryDepth>,
 }
 
 impl<T: Config> StakingLedger<T> {
@@ -470,7 +479,7 @@ impl<T: Config> StakingLedger<T> {
 			total: Zero::zero(),
 			active: Zero::zero(),
 			unlocking: Default::default(),
-			claimed_rewards: vec![],
+			claimed_rewards: Default::default(),
 		}
 	}
 
@@ -676,7 +685,9 @@ impl<T: Config> StakingLedger<T> {
 }
 
 /// A record of the nominations made by a specific account.
-#[derive(PartialEqNoBound, EqNoBound, Clone, Encode, Decode, RuntimeDebugNoBound, TypeInfo)]
+#[derive(
+	PartialEqNoBound, EqNoBound, Clone, Encode, Decode, RuntimeDebugNoBound, TypeInfo, MaxEncodedLen,
+)]
 #[codec(mel_bound())]
 #[scale_info(skip_type_params(T))]
 pub struct Nominations<T: Config> {
@@ -850,7 +861,7 @@ impl<Balance: AtLeast32BitUnsigned + Clone, T: Get<&'static PiecewiseLinear<'sta
 }
 
 /// Mode of era-forcing.
-#[derive(Copy, Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug, TypeInfo)]
+#[derive(Copy, Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)]
 #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
 pub enum Forcing {
 	/// Not forcing anything - just let whatever happen.
@@ -874,7 +885,7 @@ impl Default for Forcing {
 // A value placed in storage that represents the current version of the Staking storage. This value
 // is used by the `on_runtime_upgrade` logic to determine whether we run storage migration logic.
 // This should match directly with the semantic versions of the Rust crate.
-#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo)]
+#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
 enum Releases {
 	V1_0_0Ancient,
 	V2_0_0,
@@ -887,6 +898,7 @@ enum Releases {
 	V9_0_0,  // inject validators into `VoterList` as well.
 	V10_0_0, // remove `EarliestUnappliedSlash`.
 	V11_0_0, // Move pallet storage prefix, e.g. BagsList -> VoterBagsList
+	V12_0_0, // remove `HistoryDepth`.
 }
 
 impl Default for Releases {

frame/staking/src/migrations.rs

@@ -20,6 +20,53 @@ use super::*;
 use frame_election_provider_support::SortedListProvider;
 use frame_support::traits::OnRuntimeUpgrade;
 
+pub mod v12 {
+	use super::*;
+	use frame_support::{pallet_prelude::ValueQuery, storage_alias};
+
+	#[storage_alias]
+	type HistoryDepth<T: Config> = StorageValue<Pallet<T>, u32, ValueQuery>;
+
+	/// Clean up `HistoryDepth` from storage.
+	///
+	/// We will be depending on the configurable value of `HistoryDepth` post
+	/// this release.
+	pub struct MigrateToV12<T>(sp_std::marker::PhantomData<T>);
+	impl<T: Config> OnRuntimeUpgrade for MigrateToV12<T> {
+		#[cfg(feature = "try-runtime")]
+		fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+			frame_support::ensure!(
+				T::HistoryDepth::get() == HistoryDepth::<T>::get(),
+				"Provided value of HistoryDepth should be same as the existing storage value"
+			);
+
+			Ok(Default::default())
+		}
+
+		fn on_runtime_upgrade() -> frame_support::weights::Weight {
+			if StorageVersion::<T>::get() == Releases::V11_0_0 {
+				HistoryDepth::<T>::kill();
+				StorageVersion::<T>::put(Releases::V12_0_0);
+
+				log!(info, "v12 applied successfully");
+				T::DbWeight::get().reads_writes(1, 2)
+			} else {
+				log!(warn, "Skipping v12, should be removed");
+				T::DbWeight::get().reads(1)
+			}
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
+			frame_support::ensure!(
+				StorageVersion::<T>::get() == crate::Releases::V12_0_0,
+				"v12 not applied"
+			);
+			Ok(())
+		}
+	}
+}
+
 pub mod v11 {
 	use super::*;
 	use frame_support::{
@@ -82,11 +129,6 @@ pub mod v11 {
 
 		#[cfg(feature = "try-runtime")]
 		fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
-			frame_support::ensure!(
-				StorageVersion::<T>::get() == crate::Releases::V11_0_0,
-				"wrong version after the upgrade"
-			);
-
 			let old_pallet_name = N::get();
 			let new_pallet_name = <P as PalletInfoAccess>::name();