From 0ae1fbf8f73d9363b557f4d38162d9d977515f0c Mon Sep 17 00:00:00 2001 From: Wil Wade Date: Fri, 7 Jun 2024 21:57:26 +0000 Subject: [PATCH] Switch to a rotating cycle of chunk storage for simplicity and speed --- pallets/capacity/src/lib.rs | 96 +++++++++--------- .../src/migration/provider_boost_init.rs | 28 +----- pallets/capacity/src/tests/eras_tests.rs | 99 ++++++++++--------- .../capacity/src/tests/reward_pool_tests.rs | 18 ++-- 4 files changed, 112 insertions(+), 129 deletions(-) diff --git a/pallets/capacity/src/lib.rs b/pallets/capacity/src/lib.rs index d20ea92b27..d98f577e82 100644 --- a/pallets/capacity/src/lib.rs +++ b/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; @@ -219,6 +220,7 @@ pub mod pallet { type RewardPercentCap: Get; /// The number of chunks of Reward Pool history we expect to store + /// MUST be a divisor of [`Self::ProviderBoostHistoryLimit`] #[pallet::constant] type RewardPoolChunkLength: Get; } @@ -1158,70 +1160,62 @@ impl Pallet { reward_era: T::RewardEra, current_era: T::RewardEra, ) -> Result, DispatchError> { - let chunk_idx: u32 = Self::get_chunk_index_for_era(reward_era, current_era) - .ok_or(Error::::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::::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::::EraOutOfRange)?; Ok(*total_for_era) } - pub(crate) fn get_chunk_index_for_era( - era: T::RewardEra, - current_era: T::RewardEra, - ) -> Option { - 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) { - 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::::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::::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::::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::::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::::set(chunk_idx, Some(new_chunk)); // 1w } } diff --git a/pallets/capacity/src/migration/provider_boost_init.rs b/pallets/capacity/src/migration/provider_boost_init.rs index 6185e42d70..23239e9877 100644 --- a/pallets/capacity/src/migration/provider_boost_init.rs +++ b/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(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for ProviderBoostInit { fn on_runtime_upgrade() -> Weight { - let current_era_info = CurrentEraInfo::::get(); + let current_era_info = CurrentEraInfo::::get(); // 1r if current_era_info.eq(&RewardEraInfo::default()) { - // 1r - let current_block = frame_system::Pallet::::block_number(); // 1r + let current_block = frame_system::Pallet::::block_number(); // Whitelisted let era_index: T::RewardEra = 1u32.into(); CurrentEraInfo::::set(RewardEraInfo { era_index, started_at: current_block }); // 1w - CurrentEraProviderBoostTotal::::set(0u32.into()); - let chunks: u32 = - T::ProviderBoostHistoryLimit::get().saturating_div(T::RewardPoolChunkLength::get()); - (0u32..chunks).for_each(|chunk_index| { - let new_chunk: RewardPoolHistoryChunk = RewardPoolHistoryChunk::new(); - ProviderBoostRewardPools::::insert(chunk_index, new_chunk); - }); - T::DbWeight::get().reads_writes(2, 2) + CurrentEraProviderBoostTotal::::set(0u32.into()); // 1w + T::DbWeight::get().reads_writes(2, 1) } else { T::DbWeight::get().reads(1) } @@ -41,11 +31,6 @@ impl OnRuntimeUpgrade for ProviderBoostInit { } else { log::info!("CurrentEraInfo not found. Initialization should proceed."); } - if ProviderBoostRewardPools::::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 OnRuntimeUpgrade for ProviderBoostInit { let info = CurrentEraInfo::::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::::iter().count(), chunks); Ok(()) } } diff --git a/pallets/capacity/src/tests/eras_tests.rs b/pallets/capacity/src/tests/eras_tests.rs index 682526c762..2c5548d128 100644 --- a/pallets/capacity/src/tests/eras_tests.rs +++ b/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, ) { 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: ::RewardEra, total: BalanceOf) { - 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, + 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); } }) } diff --git a/pallets/capacity/src/tests/reward_pool_tests.rs b/pallets/capacity/src/tests/reward_pool_tests.rs index 9640bcd09a..b284da82ed 100644 --- a/pallets/capacity/src/tests/reward_pool_tests.rs +++ b/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 = (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 = (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 = ::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 = ((era * 100u32) + 1u32).into();