Switch to a rotating cycle of chunk storage for simplicity and speed

pallets/capacity/src/lib.rs

@@ -200,6 +200,7 @@ pub mod pallet {
 		/// The maximum number of eras over which one can claim rewards
 		/// Note that you can claim rewards even if you no longer are boosting, because you
 		/// may claim rewards for past eras up to the history limit.
+		/// MUST be a multiple of [`Self::RewardPoolChunkLength`]
 		#[pallet::constant]
 		type ProviderBoostHistoryLimit: Get<u32>;
 
@@ -219,6 +220,7 @@ pub mod pallet {
 		type RewardPercentCap: Get<Permill>;
 
 		/// The number of chunks of Reward Pool history we expect to store
+		/// MUST be a divisor of [`Self::ProviderBoostHistoryLimit`]
 		#[pallet::constant]
 		type RewardPoolChunkLength: Get<u32>;
 	}
@@ -1158,70 +1160,62 @@ impl<T: Config> Pallet<T> {
 		reward_era: T::RewardEra,
 		current_era: T::RewardEra,
 	) -> Result<BalanceOf<T>, DispatchError> {
-		let chunk_idx: u32 = Self::get_chunk_index_for_era(reward_era, current_era)
-			.ok_or(Error::<T>::EraOutOfRange)?;
+		// Make sure that the past era is not too old
+		let era_range = current_era.saturating_sub(reward_era);
+		ensure!(
+			current_era.gt(&reward_era) &&
+				era_range.le(&T::ProviderBoostHistoryLimit::get().into()),
+			Error::<T>::EraOutOfRange
+		);
+
+		let chunk_idx: u32 = Self::get_chunk_index_for_era(reward_era);
 		let reward_pool_chunk = Self::get_reward_pool_chunk(chunk_idx).unwrap_or_default(); // 1r
 		let total_for_era =
 			reward_pool_chunk.total_for_era(&reward_era).ok_or(Error::<T>::EraOutOfRange)?;
 		Ok(*total_for_era)
 	}
 
-	pub(crate) fn get_chunk_index_for_era(
-		era: T::RewardEra,
-		current_era: T::RewardEra,
-	) -> Option<u32> {
-		if era >= current_era {
-			return None;
-		}
-
+	/// Get the index of the chunk for a given era, hustory limit, and chunk length
+	/// Example with history limit of 6 and chunk length 3:
+	/// - Arrange the chuncks such that we overwrite a complete chunk only when it is not needed
+	/// - The cycle is thus era modulo (history limit + chunk length)
+	/// - `[0,1,2],[3,4,5],[6,7,8]`
+	/// - The second step is which chunk to add to:
+	/// - Divide the cycle by the chunk length and take the floor
+	/// - Floor(5 / 3) = 1
+	pub(crate) fn get_chunk_index_for_era(era: T::RewardEra) -> u32 {
 		let history_limit: u32 = T::ProviderBoostHistoryLimit::get();
-		let era_diff = current_era.saturating_sub(era);
-		if era_diff > history_limit.into() {
-			return None;
-		}
-
 		let chunk_len = T::RewardPoolChunkLength::get();
-		let chunks: u32 = history_limit.saturating_div(chunk_len);
-		(0u32..chunks).find(|&i| era_diff.le(&(chunk_len * (i + 1)).into()))
+		// Remove one because eras are 1 indexed
+		let era_u32: u32 = era.saturating_sub(One::one()).into();
+
+		// Add one chunk so that we always have the full history limit in our chunks
+		let cycle: u32 = era_u32 % history_limit.saturating_add(chunk_len);
+		cycle.saturating_div(chunk_len)
 	}
 
 	// This is where the reward pool gets updated.
-	// This inserts what was the current era and total boost amount into Reward Pool history, by
-	// removing the oldest item in each chunk, saving it for the next chunk, then
-	// inserting the new item.  If the entire history is full, it exits without an insert on the last chunk,
-	// effectively dropping the oldest item.
+	// Example with Limit 6, Chunk 2:
+	// - [0,1], [2,3], [4,5]
+	// - [6], [2,3], [4,5]
+	// - [6,7], [2,3], [4,5]
+	// - [6,7], [8], [4,5]
 	pub(crate) fn update_provider_boost_reward_pool(era: T::RewardEra, boost_total: BalanceOf<T>) {
-		let mut new_era = era;
-		let mut new_value = boost_total;
-		let chunks_total =
-			T::ProviderBoostHistoryLimit::get().saturating_div(T::RewardPoolChunkLength::get());
-		for chunk_idx in 0u32..chunks_total {
-			// ProviderBoostRewardPools should have already been initialized with empty BoundedBTrees.
-			if let Some(mut chunk) = ProviderBoostRewardPools::<T>::get(chunk_idx) {
-				if chunk.is_full() {
-					// this would return None only if the history is empty, and that clearly won't happen if the chunk is full.
-					if let Some(oldest_era) = chunk.earliest_era() {
-						// this is an ummutable borrow
-						let mut new_chunk = chunk.clone(); // have to do it this way because E0502
-						if let Some(oldest_value) = new_chunk.remove(&oldest_era) {
-							let try_result = new_chunk.try_insert(new_era, new_value);
-							if try_result.is_ok() {
-								new_era = *oldest_era;
-								new_value = oldest_value;
-							}
-							ProviderBoostRewardPools::<T>::set(chunk_idx, Some(new_chunk));
-						}
-					}
-				} else {
-					// Since it's not full, just insert it and ignore the result.
-					// The only reason this is supposed to fail is if the BoundedBTree is full, and
-					// since we're here, this shouldn't ever fail.
-					let _unused = chunk.try_insert(new_era, new_value);
-					ProviderBoostRewardPools::<T>::set(chunk_idx, Some(chunk));
-					break;
-				}
-			}
+		// Current era is this era
+		let chunk_idx: u32 = Self::get_chunk_index_for_era(era);
+		let mut new_chunk =
+			ProviderBoostRewardPools::<T>::get(chunk_idx).unwrap_or(RewardPoolHistoryChunk::new()); // 1r
+
+		// If it is full we are resetting.
+		// This assumes that the chunk length is a divisor of the history limit
+		if new_chunk.is_full() {
+			new_chunk = RewardPoolHistoryChunk::new();
+		};
+
+		if new_chunk.try_insert(era, boost_total).is_err() {
+			// Handle the error case that should never happen
 		}
+		ProviderBoostRewardPools::<T>::set(chunk_idx, Some(new_chunk)); // 1w
 	}
 }
 

pallets/capacity/src/migration/provider_boost_init.rs

@@ -1,7 +1,4 @@
-use crate::{
-	Config, CurrentEraInfo, CurrentEraProviderBoostTotal, ProviderBoostRewardPools, RewardEraInfo,
-	RewardPoolHistoryChunk,
-};
+use crate::{Config, CurrentEraInfo, CurrentEraProviderBoostTotal, RewardEraInfo};
 use frame_support::{
 	pallet_prelude::Weight,
 	traits::{Get, OnRuntimeUpgrade},
@@ -15,20 +12,13 @@ pub struct ProviderBoostInit<T: Config>(sp_std::marker::PhantomData<T>);
 
 impl<T: Config> OnRuntimeUpgrade for ProviderBoostInit<T> {
 	fn on_runtime_upgrade() -> Weight {
-		let current_era_info = CurrentEraInfo::<T>::get();
+		let current_era_info = CurrentEraInfo::<T>::get(); // 1r
 		if current_era_info.eq(&RewardEraInfo::default()) {
-			// 1r
-			let current_block = frame_system::Pallet::<T>::block_number(); // 1r
+			let current_block = frame_system::Pallet::<T>::block_number(); // Whitelisted
 			let era_index: T::RewardEra = 1u32.into();
 			CurrentEraInfo::<T>::set(RewardEraInfo { era_index, started_at: current_block }); // 1w
-			CurrentEraProviderBoostTotal::<T>::set(0u32.into());
-			let chunks: u32 =
-				T::ProviderBoostHistoryLimit::get().saturating_div(T::RewardPoolChunkLength::get());
-			(0u32..chunks).for_each(|chunk_index| {
-				let new_chunk: RewardPoolHistoryChunk<T> = RewardPoolHistoryChunk::new();
-				ProviderBoostRewardPools::<T>::insert(chunk_index, new_chunk);
-			});
-			T::DbWeight::get().reads_writes(2, 2)
+			CurrentEraProviderBoostTotal::<T>::set(0u32.into()); // 1w
+			T::DbWeight::get().reads_writes(2, 1)
 		} else {
 			T::DbWeight::get().reads(1)
 		}
@@ -41,11 +31,6 @@ impl<T: Config> OnRuntimeUpgrade for ProviderBoostInit<T> {
 		} else {
 			log::info!("CurrentEraInfo not found. Initialization should proceed.");
 		}
-		if ProviderBoostRewardPools::<T>::iter().count() == 0usize {
-			log::info!("ProviderBoostRewardPool will be updated with Era 0");
-		} else {
-			log::info!("ProviderBoostRewardPool has already been initialized.")
-		}
 		Ok(Vec::default())
 	}
 
@@ -56,9 +41,6 @@ impl<T: Config> OnRuntimeUpgrade for ProviderBoostInit<T> {
 		let info = CurrentEraInfo::<T>::get();
 		assert_eq!(info.started_at, current_block);
 		log::info!("CurrentEraInfo.started_at is set to {:?}.", info.started_at);
-		let chunks: u32 =
-			T::RewardPoolHistoryLimit::get().saturating_div(T::RewardPoolChunkLength::get());
-		assert_eq!(ProviderBoostRewardPools::<T>::iter().count(), chunks);
 		Ok(())
 	}
 }

pallets/capacity/src/tests/eras_tests.rs

@@ -5,7 +5,6 @@ use crate::{
 };
 use common_primitives::msa::MessageSourceId;
 use frame_support::assert_ok;
-use sp_core::Get;
 
 pub fn boost_provider_and_run_to_end_of_era(
 	staker: u64,
@@ -49,23 +48,30 @@ fn assert_chunk_is_full_and_has_earliest_era_total(
 	total: BalanceOf<Test>,
 ) {
 	let chunk = Capacity::get_reward_pool_chunk(chunk_index).unwrap();
-	assert_eq!(chunk.is_full(), is_full);
-	assert_eq!(chunk.earliest_era(), Some(&era));
-	assert_eq!(chunk.total_for_era(&era), Some(&total));
+	assert_eq!(chunk.is_full(), is_full, "{:?}", chunk);
+	assert_eq!(chunk.earliest_era(), Some(&era), "{:?}", chunk);
+	assert_eq!(chunk.total_for_era(&era), Some(&total), "{:?}", chunk);
 }
 
 // gets the last (i.e. latest non-current) stored reward pool era, which is in chunk 0.
 // asserts that it is the same as `era`, and that it has amount `total`
 fn assert_last_era_total(era: <Test as Config>::RewardEra, total: BalanceOf<Test>) {
-	let chunk = Capacity::get_reward_pool_chunk(0).unwrap();
+	let chunk_idx = Capacity::get_chunk_index_for_era(era);
+	let chunk_opt = Capacity::get_reward_pool_chunk(chunk_idx);
+	assert!(chunk_opt.is_some(), "No pool for Era: {:?} with chunk index: {:?}", era, chunk_idx);
+	let chunk = chunk_opt.unwrap();
 	let (_earliest, latest) = chunk.era_range();
 	assert_eq!(latest, era);
 	assert_eq!(chunk.total_for_era(&era), Some(&total));
 }
 
 fn assert_chunk_is_empty(chunk_index: u32) {
-	let chunk = Capacity::get_reward_pool_chunk(chunk_index).unwrap();
-	assert!(chunk.earliest_era().is_none());
+	let chunk_opt = Capacity::get_reward_pool_chunk(chunk_index);
+	if chunk_opt.is_some() {
+		assert!(chunk_opt.unwrap().earliest_era().is_none())
+	} else {
+		assert!(chunk_opt.is_none())
+	}
 }
 
 // Test that additional stake is carried over to the next era's RewardPoolInfo.
@@ -91,27 +97,39 @@ fn start_new_era_if_needed_updates_reward_pool() {
 		for i in [4u32, 5u32, 6u32] {
 			boost_provider_and_run_to_end_of_era(staker, provider_msa, stake_amount, i);
 		}
-		assert_chunk_is_full_and_has_earliest_era_total(0, true, 4, 400);
-		assert_chunk_is_full_and_has_earliest_era_total(1, true, 1, 100);
+		// No change
+		assert_chunk_is_full_and_has_earliest_era_total(0, true, 1, 100);
+		// New Chunk
+		assert_chunk_is_full_and_has_earliest_era_total(1, true, 4, 400);
 		assert_chunk_is_empty(2);
 		assert_last_era_total(6, 600);
 
-		for i in [7u32, 8u32, 9u32] {
+		for i in [7u32, 8u32, 9u32, 10u32, 11u32, 12u32, 13u32, 14u32, 15u32] {
 			boost_provider_and_run_to_end_of_era(staker, provider_msa, stake_amount, i);
 		}
-		assert_chunk_is_full_and_has_earliest_era_total(0, true, 7, 700);
+		// No changes
+		assert_chunk_is_full_and_has_earliest_era_total(0, true, 1, 100);
 		assert_chunk_is_full_and_has_earliest_era_total(1, true, 4, 400);
-		assert_chunk_is_full_and_has_earliest_era_total(2, true, 1, 100);
-		assert_last_era_total(9, 900);
+		// New
+		assert_chunk_is_full_and_has_earliest_era_total(2, true, 7, 700);
+		assert_chunk_is_full_and_has_earliest_era_total(3, true, 10, 1000);
+		assert_chunk_is_full_and_has_earliest_era_total(4, true, 13, 1300);
+		assert_last_era_total(15, 1500);
 
 		// check that it all rolls over properly.
-		for i in [10u32, 11u32] {
+		for i in [16u32, 17u32] {
 			boost_provider_and_run_to_end_of_era(staker, provider_msa, stake_amount, i);
 		}
-		assert_chunk_is_full_and_has_earliest_era_total(0, true, 9, 900);
-		assert_chunk_is_full_and_has_earliest_era_total(1, true, 6, 600);
-		assert_chunk_is_full_and_has_earliest_era_total(2, true, 3, 300);
-		assert_last_era_total(11, 1100);
+		// No Changes
+		assert_chunk_is_full_and_has_earliest_era_total(1, true, 4, 400);
+		assert_chunk_is_full_and_has_earliest_era_total(2, true, 7, 700);
+		assert_chunk_is_full_and_has_earliest_era_total(3, true, 10, 1000);
+		assert_chunk_is_full_and_has_earliest_era_total(4, true, 13, 1300);
+		// New
+		assert_chunk_is_full_and_has_earliest_era_total(0, false, 16, 1600);
+		assert_last_era_total(17, 1700);
+		// There shouldn't be a chunk 5 ever with this config
+		assert_chunk_is_empty(5);
 	});
 }
 
@@ -138,42 +156,31 @@ fn get_expiration_block_for_era_works() {
 #[test]
 fn get_chunk_index_for_era_works() {
 	new_test_ext().execute_with(|| {
+		#[derive(Debug)]
 		struct TestCase {
 			era: u32,
-			current_era: u32,
-			expected: Option<u32>,
+			expected: u32,
 		}
 		// assuming history limit is 12, chunk length is 3
+		// [1,2,3],[4,5,6],[7,8,9],[10,11,12],[13,14,15]
+		// [16],[4,5,6],[7,8,9],[10,11,12],[13,14,15]
 		for test in {
 			vec![
-				TestCase { era: 3, current_era: 6, expected: Some(0) },
-				TestCase { era: 2, current_era: 1, expected: None },
-				TestCase { era: 3, current_era: 16, expected: None },
-				TestCase { era: 1, current_era: 1, expected: None },
-				TestCase { era: 1, current_era: 2, expected: Some(0) },
-				TestCase { era: 1, current_era: 3, expected: Some(0) },
-				TestCase { era: 1, current_era: 4, expected: Some(0) },
-				TestCase { era: 1, current_era: 5, expected: Some(1) },
-				TestCase { era: 1, current_era: 6, expected: Some(1) },
-				TestCase { era: 1, current_era: 7, expected: Some(1) },
-				TestCase { era: 12, current_era: 13, expected: Some(0) },
-				TestCase { era: 11, current_era: 13, expected: Some(0) },
-				TestCase { era: 10, current_era: 13, expected: Some(0) },
-				TestCase { era: 9, current_era: 13, expected: Some(1) },
-				TestCase { era: 8, current_era: 13, expected: Some(1) },
-				TestCase { era: 7, current_era: 13, expected: Some(1) },
-				TestCase { era: 6, current_era: 13, expected: Some(2) },
-				TestCase { era: 5, current_era: 13, expected: Some(2) },
-				TestCase { era: 4, current_era: 13, expected: Some(2) },
-				TestCase { era: 3, current_era: 13, expected: Some(3) },
-				TestCase { era: 55, current_era: 60, expected: Some(1) },
-				TestCase { era: 55, current_era: 62, expected: Some(2) },
+				TestCase { era: 1, expected: 0 },
+				TestCase { era: 2, expected: 0 },
+				TestCase { era: 3, expected: 0 },
+				TestCase { era: 4, expected: 1 },
+				TestCase { era: 5, expected: 1 },
+				TestCase { era: 6, expected: 1 },
+				TestCase { era: 7, expected: 2 },
+				TestCase { era: 11, expected: 3 },
+				TestCase { era: 15, expected: 4 },
+				TestCase { era: 16, expected: 0 },
+				TestCase { era: 22, expected: 2 },
+				TestCase { era: 55, expected: 3 },
 			]
 		} {
-			assert_eq!(
-				Capacity::get_chunk_index_for_era(test.era, test.current_era),
-				test.expected
-			);
+			assert_eq!(Capacity::get_chunk_index_for_era(test.era), test.expected, "{:?}", test);
 		}
 	})
 }

pallets/capacity/src/tests/reward_pool_tests.rs

@@ -86,8 +86,8 @@ fn get_total_stake_for_past_era_works_with_1_full_chunk() {
 	new_test_ext().execute_with(|| {
 		System::set_block_number(52);
 		set_era_and_reward_pool(6, 51, 1000);
-		fill_reward_pool_history_chunk(1, 1, 2, 100); // eras 1-3
-		fill_reward_pool_history_chunk(0, 3, 3, 300); // eras 4,5
+		fill_reward_pool_history_chunk(0, 1, 3, 100); // eras 1-3
+		fill_reward_pool_history_chunk(1, 4, 2, 400); // eras 4,5
 		for i in 3u32..=5u32 {
 			let expected_total: BalanceOf<Test> = (i * 100u32).into();
 			let actual = Capacity::get_total_stake_for_past_era(i, 6);
@@ -102,9 +102,9 @@ fn get_total_stake_for_past_era_works_with_2_full_chunks() {
 	new_test_ext().execute_with(|| {
 		System::set_block_number(72);
 		set_era_and_reward_pool(8, 71, 1000);
-		fill_reward_pool_history_chunk(2, 1, 1, 100);
-		fill_reward_pool_history_chunk(1, 2, 3, 200);
-		fill_reward_pool_history_chunk(0, 5, 3, 500);
+		fill_reward_pool_history_chunk(0, 1, 3, 100);
+		fill_reward_pool_history_chunk(1, 4, 3, 400);
+		fill_reward_pool_history_chunk(2, 7, 1, 700);
 		for i in 1u32..=7u32 {
 			let expected_total: BalanceOf<Test> = (i * 100u32).into();
 			assert_eq!(Capacity::get_total_stake_for_past_era(i, 8), Ok(expected_total));
@@ -120,10 +120,10 @@ fn get_total_stake_for_past_era_works_with_full_reward_pool() {
 		let history_limit: u32 = <Test as Config>::ProviderBoostHistoryLimit::get();
 		set_era_and_reward_pool(13, 121, (2000u32).into());
 
-		fill_reward_pool_history_chunk(3, 1, 3, 101);
-		fill_reward_pool_history_chunk(2, 4, 3, 401);
-		fill_reward_pool_history_chunk(1, 7, 3, 701);
-		fill_reward_pool_history_chunk(0, 10, 3, 1001);
+		fill_reward_pool_history_chunk(0, 1, 3, 101);
+		fill_reward_pool_history_chunk(1, 4, 3, 401);
+		fill_reward_pool_history_chunk(2, 7, 3, 701);
+		fill_reward_pool_history_chunk(3, 10, 3, 1001);
 
 		(1u32..=history_limit).for_each(|era| {
 			let expected_total: BalanceOf<Test> = ((era * 100u32) + 1u32).into();