refactor and doc improve

Author: Ank4n

frame/staking/src/migrations.rs

@@ -27,9 +27,10 @@ pub mod v11 {
 	#[storage_alias]
 	type HistoryDepth<T: Config> = StorageValue<Pallet<T>, u32, ValueQuery>;
 
-	/// Clean up History Depth from storage
+	/// Clean up `HistoryDepth` from storage
 	///
-	/// We will be depending on the configurable value of History Depth post this release.
+	/// We will be depending on the configurable value of `HistoryDepth` post
+	/// this release.
 	pub struct MigrateToV11<T>(sp_std::marker::PhantomData<T>);
 	impl<T: Config> OnRuntimeUpgrade for MigrateToV11<T> {
 		fn on_runtime_upgrade() -> frame_support::weights::Weight {

frame/staking/src/pallet/impls.rs

@@ -100,7 +100,7 @@ impl<T: Config> Pallet<T> {
 			Error::<T>::InvalidEraToReward
 				.with_weight(T::WeightInfo::payout_stakers_alive_staked(0))
 		})?;
-		let history_depth = Self::history_depth();
+		let history_depth = T::HistoryDepth::get();
 		ensure!(
 			era <= current_era && era >= current_era.saturating_sub(history_depth),
 			Error::<T>::InvalidEraToReward
@@ -131,7 +131,8 @@ impl<T: Config> Pallet<T> {
 				.claimed_rewards
 				.try_insert(pos, era)
 				// Since we retain era entries in `claimed_rewards` only upto
-				// `HistoryDepth`, following bound is always expected to be satisfied.
+				// `HistoryDepth`, following bound is always expected to be
+				// satisfied.
 				.defensive_map_err(|_| Error::<T>::BoundNotMet)?,
 		}
 
@@ -422,7 +423,7 @@ impl<T: Config> Pallet<T> {
 		ErasStartSessionIndex::<T>::insert(&new_planned_era, &start_session_index);
 
 		// Clean old era information.
-		if let Some(old_era) = new_planned_era.checked_sub(Self::history_depth() + 1) {
+		if let Some(old_era) = new_planned_era.checked_sub(T::HistoryDepth::get() + 1) {
 			Self::clear_era_information(old_era);
 		}
 
@@ -876,15 +877,6 @@ impl<T: Config> Pallet<T> {
 			DispatchClass::Mandatory,
 		);
 	}
-
-	/// This will return the currently configured `HistoryDepth`
-	///
-	/// With release of v11, `HistoryDepth` is migrated to a configurable
-	/// value instead of being a storage item. This function replaces the
-	/// old fn history_depth that used to read from the storage.
-	pub fn history_depth() -> u32 {
-		T::HistoryDepth::get()
-	}
 }
 
 impl<T: Config> ElectionDataProvider for Pallet<T> {

frame/staking/src/pallet/mod.rs

@@ -125,15 +125,16 @@ pub mod pallet {
 		#[pallet::constant]
 		type MaxNominations: Get<u32>;
 
-		/// `HistoryDepth` stores the number of previous eras for which
-		/// we keep track of validator `claimed_rewards`.
-		///
-		/// Since `HistoryDepth` is used as a bound for BoundedVec
-		/// `claimed_rewards`, we should use extreme caution while
-		/// changing this value once it has been already deployed in
-		/// production. In general, increasing this value should be okay
-		/// but decreasing it will cause inconsistencies and needs to be
-		/// handled by doing migration of `StakingLedger.claimed_rewards`.
+		/// `HistoryDepth` is the number of eras to keep in history.
+		///
+		/// This used to be a storage value previously. If you are migrating
+		/// from storage value to config value, you should read this value from
+		/// storage and set the same value in configuration.
+		///
+		/// Note: `HistoryDepth` is used as the upper bound for the `BoundedVec`
+		/// item `StakingLedger.claimed_rewards`. Setting this value lower than
+		/// the existing value can lead to inconsistencies and will need to be
+		/// handled properly in migration.
 		#[pallet::constant]
 		type HistoryDepth: Get<u32>;
 
@@ -819,8 +820,9 @@ pub mod pallet {
 				unlocking: Default::default(),
 				claimed_rewards: (last_reward_era..current_era)
 					.try_collect()
-					// Since last_reward_era is calculated as `current_era - HistoryDepth`,
-					// following bound is always expected to be satisfied.
+					// Since last_reward_era is calculated as `current_era -
+					// HistoryDepth`, following bound is always expected to be
+					// satisfied.
 					.defensive_map_err(|_| Error::<T>::BoundNotMet)?,
 			};
 			Self::update_ledger(&controller, &item);

frame/staking/src/tests.rs

@@ -3347,15 +3347,15 @@ fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() {
 		assert!(total_payout_2 != total_payout_0);
 		assert!(total_payout_2 != total_payout_1);
 
-		mock::start_active_era(Staking::history_depth() + 1);
+		mock::start_active_era(HistoryDepth::get() + 1);
 
 		let active_era = active_era();
 
 		// This is the latest planned era in staking, not the active era
 		let current_era = Staking::current_era().unwrap();
 
 		// Last kept is 1:
-		assert!(current_era - Staking::history_depth() == 1);
+		assert!(current_era - HistoryDepth::get() == 1);
 		assert_noop!(
 			Staking::payout_stakers(Origin::signed(1337), 11, 0),
 			// Fail: Era out of history