Skip to content

Commit

Permalink
Boosting: removed added getters (#2112)
Browse files Browse the repository at this point in the history
# Goal
The goal of this PR is to removed added getters

Related to #2105
  • Loading branch information
aramikm authored Aug 5, 2024
1 parent 40f3910 commit 65df923
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 56 deletions.
13 changes: 5 additions & 8 deletions designdocs/provider_boosting_implementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ pub struct StakingDetails<T: Config> {
```rust
/// Individual history for each account that has Provider-Boosted.
#[pallet::storage]
#[pallet::getter(fn get_staking_history_for)]
pub type ProviderBoostHistories<T: Config> =
StorageMap<_, Twox64Concat, T::AccountId, ProviderBoostHistory<T>>;
```
Expand All @@ -102,7 +101,7 @@ Those who wish to do both types of staking must use two different accounts, one
#### unstake

The unstake parameters are the same, and unstake behavior is the same for `MaximumCapacity` as before, however
for a `ProviderBoost` staker-target relationship, the behavior must be different. The Provider-Boost account must
for a `ProviderBoost` staker-target relationship, the behavior must be different. The Provider-Boost account must
first claim all unpaid rewards before an unstake can succeed.

```rust
Expand Down Expand Up @@ -140,7 +139,7 @@ pub trait ProviderBoostRewardsProvider<T: Config> {
fn staking_reward_totals(
account_id: Self::AccountId,
) -> Result<BalanceOf<T>, DispatchError>;

/// Calculate the reward for a single era. We don't care about the era number,
/// just the values.
fn era_staking_reward(
Expand Down Expand Up @@ -194,15 +193,14 @@ pub trait Config: frame_system::Config {
### NEW: ProviderBoostRewardPools, CurrentEraProviderBoostTotal
The storage of the total amount staked for the ProviderBoostHistoryLimit number of eras is divided into chunks of
BoundedBTreeMaps, which store Key = RewardEra, Value = Total stake for that era. The chunks are updated in rotating fashion
in order to minimize reads and writes for listing individual rewards, claiming individual rewards, and changing to a
in order to minimize reads and writes for listing individual rewards, claiming individual rewards, and changing to a
new Reward Era, when necessary, during `on_initialize`.

```rust
/// Reward Pool history is divided into chunks of size RewardPoolChunkLength.
/// ProviderBoostHistoryLimit is the total number of items, the key is the
/// chunk number.
#[pallet::storage]
#[pallet::getter(fn get_reward_pool_chunk)]
pub type ProviderBoostRewardPools<T: Config> =
StorageMap<_, Twox64Concat, u32, RewardPoolHistoryChunk<T>>;

Expand All @@ -219,7 +217,6 @@ Storage is whitelisted because it's accessed every block and would improperly ad
```rust
#[pallet::storage]
#[pallet::whitelist_storage]
#[pallet::getter(fn get_current_era)]
/// Similar to CurrentEpoch
pub type CurrentEraInfo<T:Config> = StorageValue<_, T::RewardEraInfo, ValueQuery>;

Expand Down Expand Up @@ -249,7 +246,7 @@ pub enum Error<T> {
MustFirstClaimRewards,
/// Too many change_staking_target calls made in this RewardEra.
MaxRetargetsExceeded,
/// Account either has no staking account at all or it is not a ProviderBoost type
/// Account either has no staking account at all or it is not a ProviderBoost type
NotAProviderBoostAccount,
}
```
Expand Down Expand Up @@ -281,7 +278,7 @@ The event `ProviderBoostRewardClaimed` is emitted with the parameters of the ext


```rust
/// Claim all outstanding Provider Boost rewards, up to ProviderBoostHistoryLimit Reward Eras
/// Claim all outstanding Provider Boost rewards, up to ProviderBoostHistoryLimit Reward Eras
/// in the past. Accounts should check for unclaimed rewards before calling this extrinsic
/// to avoid needless transaction fees.
/// Errors:
Expand Down
2 changes: 1 addition & 1 deletion pallets/capacity/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ benchmarks! {
}: {
Capacity::<T>::start_new_reward_era_if_needed(current_block);
} verify {
let new_era_info = Capacity::<T>::get_current_era();
let new_era_info = CurrentEraInfo::<T>::get();
assert_eq!(current_era.saturating_add(1u32.into()), new_era_info.era_index);
assert_eq!(current_block, new_era_info.started_at);
}
Expand Down
33 changes: 15 additions & 18 deletions pallets/capacity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,21 +251,18 @@ pub mod pallet {

/// stores how many times an account has retargeted, and when it last retargeted.
#[pallet::storage]
#[pallet::getter(fn get_retargets_for)]
pub type Retargets<T: Config> = StorageMap<_, Twox64Concat, T::AccountId, RetargetInfo<T>>;

/// Information about the current reward era. Checked every block.
#[pallet::storage]
#[pallet::whitelist_storage]
#[pallet::getter(fn get_current_era)]
pub type CurrentEraInfo<T: Config> =
StorageValue<_, RewardEraInfo<RewardEra, BlockNumberFor<T>>, ValueQuery>;

/// Reward Pool history is divided into chunks of size RewardPoolChunkLength.
/// ProviderBoostHistoryLimit is the total number of items, the key is the
/// chunk number.
#[pallet::storage]
#[pallet::getter(fn get_reward_pool_chunk)]
pub type ProviderBoostRewardPools<T: Config> =
StorageMap<_, Twox64Concat, u32, RewardPoolHistoryChunk<T>>;

Expand All @@ -275,7 +272,6 @@ pub mod pallet {

/// Individual history for each account that has Provider-Boosted.
#[pallet::storage]
#[pallet::getter(fn get_staking_history_for)]
pub type ProviderBoostHistories<T: Config> =
StorageMap<_, Twox64Concat, T::AccountId, ProviderBoostHistory<T>>;

Expand Down Expand Up @@ -754,7 +750,7 @@ impl<T: Config> Pallet<T> {
capacity_details.deposit(amount, &capacity).ok_or(ArithmeticError::Overflow)?;
Self::set_capacity_for(*target, capacity_details);

let era = Self::get_current_era().era_index;
let era = CurrentEraInfo::<T>::get().era_index;
Self::upsert_boost_history(staker, era, *amount, true)?;

let reward_pool_total = CurrentEraProviderBoostTotal::<T>::get();
Expand Down Expand Up @@ -824,7 +820,7 @@ impl<T: Config> Pallet<T> {

let staking_type = staking_account.staking_type;
if staking_type == StakingType::ProviderBoost {
let era = Self::get_current_era().era_index;
let era = CurrentEraInfo::<T>::get().era_index;
Self::upsert_boost_history(&unstaker, era, actual_unstaked_amount, false)?;
let reward_pool_total = CurrentEraProviderBoostTotal::<T>::get();
CurrentEraProviderBoostTotal::<T>::set(
Expand Down Expand Up @@ -977,7 +973,8 @@ impl<T: Config> Pallet<T> {
}

fn start_new_reward_era_if_needed(current_block: BlockNumberFor<T>) -> Weight {
let current_era_info: RewardEraInfo<RewardEra, BlockNumberFor<T>> = Self::get_current_era(); // 1r
let current_era_info: RewardEraInfo<RewardEra, BlockNumberFor<T>> =
CurrentEraInfo::<T>::get(); // 1r

if current_block.saturating_sub(current_era_info.started_at) >= T::EraLength::get().into() {
// 1r
Expand All @@ -1003,8 +1000,8 @@ impl<T: Config> Pallet<T> {
/// Returns:
/// Error::MaxRetargetsExceeded if they try to retarget too many times in one era.
fn update_retarget_record(staker: &T::AccountId) -> Result<(), DispatchError> {
let current_era: RewardEra = Self::get_current_era().era_index;
let mut retargets = Self::get_retargets_for(staker).unwrap_or_default();
let current_era: RewardEra = CurrentEraInfo::<T>::get().era_index;
let mut retargets = Retargets::<T>::get(staker).unwrap_or_default();
ensure!(retargets.update(current_era).is_some(), Error::<T>::MaxRetargetsExceeded);
Retargets::<T>::set(staker, Some(retargets));
Ok(())
Expand Down Expand Up @@ -1045,7 +1042,7 @@ impl<T: Config> Pallet<T> {
boost_amount: BalanceOf<T>,
add: bool,
) -> Result<(), DispatchError> {
let mut boost_history = Self::get_staking_history_for(account).unwrap_or_default();
let mut boost_history = ProviderBoostHistories::<T>::get(account).unwrap_or_default();

let upsert_result = if add {
boost_history.add_era_balance(&current_era, &boost_amount)
Expand All @@ -1061,8 +1058,8 @@ impl<T: Config> Pallet<T> {
}

pub(crate) fn has_unclaimed_rewards(account: &T::AccountId) -> bool {
let current_era = Self::get_current_era().era_index;
match Self::get_staking_history_for(account) {
let current_era = CurrentEraInfo::<T>::get().era_index;
match ProviderBoostHistories::<T>::get(account) {
Some(provider_boost_history) => {
match provider_boost_history.count() {
0usize => false,
Expand Down Expand Up @@ -1098,10 +1095,10 @@ impl<T: Config> Pallet<T> {
return Ok(BoundedVec::new());
}

let staking_history =
Self::get_staking_history_for(account).ok_or(Error::<T>::NotAProviderBoostAccount)?; // cached read
let staking_history = ProviderBoostHistories::<T>::get(account)
.ok_or(Error::<T>::NotAProviderBoostAccount)?; // cached read

let current_era_info = Self::get_current_era(); // cached read, ditto
let current_era_info = CurrentEraInfo::<T>::get(); // cached read, ditto
let max_history: u32 = T::ProviderBoostHistoryLimit::get();

let mut reward_era = current_era_info.era_index.saturating_sub((max_history).into());
Expand Down Expand Up @@ -1152,7 +1149,7 @@ impl<T: Config> Pallet<T> {
// Returns the block number for the end of the provided era. Assumes `era` is at least this
// era or in the future
pub(crate) fn block_at_end_of_era(era: RewardEra) -> BlockNumberFor<T> {
let current_era_info = Self::get_current_era();
let current_era_info = CurrentEraInfo::<T>::get();
let era_length: BlockNumberFor<T> = T::EraLength::get().into();

let era_diff = if current_era_info.era_index.eq(&era) {
Expand All @@ -1177,7 +1174,7 @@ impl<T: Config> Pallet<T> {
);

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 reward_pool_chunk = ProviderBoostRewardPools::<T>::get(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)
Expand Down Expand Up @@ -1236,7 +1233,7 @@ impl<T: Config> Pallet<T> {
let mut new_history: ProviderBoostHistory<T> = ProviderBoostHistory::new();
let last_staked_amount =
rewards.last().unwrap_or(&UnclaimedRewardInfo::default()).staked_amount;
let current_era = Self::get_current_era().era_index;
let current_era = CurrentEraInfo::<T>::get().era_index;
// We have already paid out for the previous era. Put one entry for the previous era as if that is when they staked,
// so they will be credited for current_era.
ensure!(
Expand Down
12 changes: 6 additions & 6 deletions pallets/capacity/src/tests/claim_staking_rewards_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use crate::{
},
testing_utils::{run_to_block, setup_provider},
},
Error, Event,
CurrentEraInfo, Error, Event,
Event::ProviderBoostRewardClaimed,
MessageSourceId,
MessageSourceId, ProviderBoostHistories,
StakingType::*,
};
use frame_support::{assert_noop, assert_ok};
Expand All @@ -21,9 +21,9 @@ fn claim_staking_rewards_leaves_one_history_item_for_current_era() {

setup_provider(&account, &target, &amount, ProviderBoost);
run_to_block(31);
assert_eq!(Capacity::get_current_era().era_index, 4u32);
assert_eq!(CurrentEraInfo::<Test>::get().era_index, 4u32);

let current_history = Capacity::get_staking_history_for(account).unwrap();
let current_history = ProviderBoostHistories::<Test>::get(account).unwrap();
assert_eq!(current_history.count(), 1usize);
let history_item = current_history.get_entry_for_era(&1u32).unwrap();
assert_eq!(*history_item, amount);
Expand Down Expand Up @@ -57,7 +57,7 @@ fn claim_staking_rewards_mints_and_transfers_expected_total() {

setup_provider(&account, &target, &amount, ProviderBoost);
run_to_block(31);
assert_eq!(Capacity::get_current_era().era_index, 4u32);
assert_eq!(CurrentEraInfo::<Test>::get().era_index, 4u32);
assert_ok!(Capacity::claim_staking_rewards(RuntimeOrigin::signed(account)));
System::assert_last_event(
Event::<Test>::ProviderBoostRewardClaimed { account, reward_amount: 8u64 }.into(),
Expand All @@ -70,7 +70,7 @@ fn claim_staking_rewards_mints_and_transfers_expected_total() {
assert_transferable::<Test>(&account, 8u64);

run_to_block(51);
assert_eq!(Capacity::get_current_era().era_index, 6u32);
assert_eq!(CurrentEraInfo::<Test>::get().era_index, 6u32);
assert_ok!(Capacity::claim_staking_rewards(RuntimeOrigin::signed(account)));
System::assert_last_event(
ProviderBoostRewardClaimed { account, reward_amount: 8u64 }.into(),
Expand Down
11 changes: 6 additions & 5 deletions pallets/capacity/src/tests/eras_tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::mock::*;
use crate::{
tests::testing_utils::*, BalanceOf, CurrentEraInfo, CurrentEraProviderBoostTotal, RewardEraInfo,
tests::testing_utils::*, BalanceOf, CurrentEraInfo, CurrentEraProviderBoostTotal,
ProviderBoostRewardPools, RewardEraInfo,
};
use common_primitives::{capacity::RewardEra, msa::MessageSourceId};
use frame_support::assert_ok;
Expand Down Expand Up @@ -46,7 +47,7 @@ fn assert_chunk_is_full_and_has_earliest_era_total(
era: RewardEra,
total: BalanceOf<Test>,
) {
let chunk = Capacity::get_reward_pool_chunk(chunk_index).unwrap();
let chunk = ProviderBoostRewardPools::<Test>::get(chunk_index).unwrap();
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);
Expand All @@ -56,7 +57,7 @@ fn assert_chunk_is_full_and_has_earliest_era_total(
// asserts that it is the same as `era`, and that it has amount `total`
fn assert_last_era_total(era: RewardEra, total: BalanceOf<Test>) {
let chunk_idx = Capacity::get_chunk_index_for_era(era);
let chunk_opt = Capacity::get_reward_pool_chunk(chunk_idx);
let chunk_opt = ProviderBoostRewardPools::<Test>::get(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();
Expand All @@ -65,7 +66,7 @@ fn assert_last_era_total(era: RewardEra, total: BalanceOf<Test>) {
}

fn assert_chunk_is_empty(chunk_index: u32) {
let chunk_opt = Capacity::get_reward_pool_chunk(chunk_index);
let chunk_opt = ProviderBoostRewardPools::<Test>::get(chunk_index);
if chunk_opt.is_some() {
assert!(chunk_opt.unwrap().earliest_era().is_none())
} else {
Expand Down Expand Up @@ -135,7 +136,7 @@ fn start_new_era_if_needed_updates_reward_pool() {
#[test]
fn get_expiration_block_for_era_works() {
new_test_ext().execute_with(|| {
assert_eq!(Capacity::get_current_era().era_index, 1u32);
assert_eq!(CurrentEraInfo::<Test>::get().era_index, 1u32);
assert_eq!(Capacity::block_at_end_of_era(10u32), 100);

set_era_and_reward_pool(7, 61, 0);
Expand Down
10 changes: 5 additions & 5 deletions pallets/capacity/src/tests/provider_boost_history_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
mock::{new_test_ext, Capacity, RuntimeOrigin, Test},
testing_utils::{run_to_block, setup_provider, system_run_to_block},
},
Config, ProviderBoostHistory,
Config, ProviderBoostHistories, ProviderBoostHistory,
StakingType::{MaximumCapacity, ProviderBoost},
};
use common_primitives::capacity::RewardEra;
Expand All @@ -17,7 +17,7 @@ fn provider_boost_adds_to_staking_history() {
let target = 1u64;

setup_provider(&staker, &target, &1_000u64, ProviderBoost);
let history = Capacity::get_staking_history_for(staker);
let history = ProviderBoostHistories::<Test>::get(staker);
assert!(history.is_some());
})
}
Expand All @@ -32,7 +32,7 @@ fn multiple_provider_boosts_updates_history_correctly() {
assert_ok!(Capacity::provider_boost(RuntimeOrigin::signed(staker), target, 200));

// should update era 1 history
let mut history = Capacity::get_staking_history_for(staker).unwrap();
let mut history = ProviderBoostHistories::<Test>::get(staker).unwrap();
assert_eq!(history.count(), 1);
assert_eq!(history.get_entry_for_era(&1u32).unwrap(), &700u64);

Expand All @@ -42,7 +42,7 @@ fn multiple_provider_boosts_updates_history_correctly() {
assert_ok!(Capacity::provider_boost(RuntimeOrigin::signed(staker), target, 200));

// should add an era 2 history
history = Capacity::get_staking_history_for(staker).unwrap();
history = ProviderBoostHistories::<Test>::get(staker).unwrap();
assert_eq!(history.count(), 2);
assert_eq!(history.get_entry_for_era(&2u32).unwrap(), &900u64);
})
Expand All @@ -55,7 +55,7 @@ fn stake_does_not_add_to_staking_history() {
let target = 1u64;

setup_provider(&staker, &target, &1_000u64, MaximumCapacity);
let history = Capacity::get_staking_history_for(staker);
let history = ProviderBoostHistories::<Test>::get(staker);
assert!(history.is_none());
})
}
Expand Down
11 changes: 6 additions & 5 deletions pallets/capacity/src/tests/rewards_provider_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use crate::{
tests::testing_utils::{
run_to_block, set_era_and_reward_pool, setup_provider, system_run_to_block,
},
BalanceOf, Config, ProviderBoostHistories, ProviderBoostHistory, ProviderBoostRewardsProvider,
BalanceOf, Config, CurrentEraInfo, ProviderBoostHistories, ProviderBoostHistory,
ProviderBoostRewardsProvider,
StakingType::*,
UnclaimedRewardInfo,
};
Expand Down Expand Up @@ -112,8 +113,8 @@ fn list_unclaimed_rewards_has_eligible_rewards() {
assert_ok!(Capacity::provider_boost(RuntimeOrigin::signed(account), target, amount));

run_to_block(51);
assert_eq!(Capacity::get_current_era().era_index, 6u32);
assert_eq!(Capacity::get_current_era().started_at, 51u32);
assert_eq!(CurrentEraInfo::<Test>::get().era_index, 6u32);
assert_eq!(CurrentEraInfo::<Test>::get().started_at, 51u32);

// rewards for era 6 should not be returned; era 6 is current era and therefore ineligible.
// eligible amounts for rewards for eras should be: 1=0, 2=1k, 3=2k, 4=2k, 5=3k
Expand Down Expand Up @@ -198,8 +199,8 @@ fn list_unclaimed_rewards_returns_correctly_for_old_single_boost() {
assert!(!Capacity::has_unclaimed_rewards(&account));

run_to_block(131);
assert_eq!(Capacity::get_current_era().era_index, 14u32);
assert_eq!(Capacity::get_current_era().started_at, 131u32);
assert_eq!(CurrentEraInfo::<Test>::get().era_index, 14u32);
assert_eq!(CurrentEraInfo::<Test>::get().started_at, 131u32);

let rewards = Capacity::list_unclaimed_rewards(&account).unwrap();

Expand Down
Loading

0 comments on commit 65df923

Please sign in to comment.