Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

claim_staking_rewards extrinsic #2080

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions common/primitives/src/capacity.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::msa::MessageSourceId;
use frame_support::traits::tokens::Balance;
use scale_info::TypeInfo;
use sp_core::{Decode, Encode, MaxEncodedLen, RuntimeDebug};
use sp_runtime::DispatchError;

/// The type of a Reward Era
Expand Down Expand Up @@ -55,3 +57,22 @@ pub trait Replenishable {
/// Checks if an account can be replenished.
fn can_replenish(msa_id: MessageSourceId) -> bool;
}

/// Result of checking a Boost History item to see if it's eligible for a reward.
#[derive(
Copy, Clone, Default, Encode, Eq, Decode, RuntimeDebug, MaxEncodedLen, PartialEq, TypeInfo,
)]
#[scale_info(skip_type_params(T))]
pub struct UnclaimedRewardInfo<Balance, BlockNumber> {
/// The Reward Era for which this reward was earned
pub reward_era: RewardEra,
/// When this reward expires, i.e. can no longer be claimed
pub expires_at_block: BlockNumber,
/// The total staked in this era as of the current block
pub staked_amount: Balance,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to have the amount they staked in each Reward Info.

/// The amount staked in this era that is eligible for rewards. Does not count additional amounts
/// staked in this era.
pub eligible_amount: Balance,
/// The amount in token of the reward (only if it can be calculated using only on chain data)
pub earned_amount: Balance,
}
7 changes: 3 additions & 4 deletions e2e/capacity/change_staking_target.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import { getFundingSource } from '../scaffolding/funding';
import {
createKeys, createMsaAndProvider,
stakeToProvider,
CENTS, DOLLARS, createAndFundKeypair, createProviderKeysAndId
} from "../scaffolding/helpers";
CENTS, DOLLARS, createAndFundKeypair, createProviderKeysAndId, getNonce,
} from '../scaffolding/helpers';
import { KeyringPair } from '@polkadot/keyring/types';

const fundingSource = getFundingSource('capacity-replenishment');

Expand Down Expand Up @@ -39,8 +40,6 @@ describe("Capacity: change_staking_target", function() {
await assert.rejects(call.signAndSend(),
(err) => {
assert. strictEqual(err?.name, 'InvalidTarget', `expected InvalidTarget, got ${err?.name}`);
// // {name: "InvalidTarget"}
// assert. strictEqual(err?.message, `Wrong value: expected`);
return true;
});
});
Expand Down
49 changes: 35 additions & 14 deletions pallets/capacity/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,23 @@ fn fill_unlock_chunks<T: Config>(caller: &T::AccountId, count: u32) {
UnstakeUnlocks::<T>::set(caller, Some(unlocking));
}

fn fill_reward_pool_chunks<T: Config>() {
let chunk_len = T::RewardPoolChunkLength::get();
let chunks = T::ProviderBoostHistoryLimit::get() / (chunk_len);
for i in 0..chunks {
let mut new_chunk = RewardPoolHistoryChunk::<T>::new();
for j in 0..chunk_len {
let era = (i + 1) * (j + 1);
assert_ok!(new_chunk.try_insert(era.into(), (1000u32 * era).into()));
}
ProviderBoostRewardPools::<T>::set(i, Some(new_chunk));
fn fill_reward_pool_chunks<T: Config>(current_era: RewardEra) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous version of this function, while it worked for the provider_boost benchmark, did not correctly fill up the reward_pool_chunks, so it caused an EraOutOfRange error in the claim_staking_reward benchmark.

let history_limit: RewardEra = <T as Config>::ProviderBoostHistoryLimit::get();
let starting_era: RewardEra = current_era - history_limit - 1u32;
for era in starting_era..current_era {
Capacity::<T>::update_provider_boost_reward_pool(era, 10_000u32.into());
}
}

fn fill_boost_history<T: Config>(
caller: &T::AccountId,
amount: BalanceOf<T>,
current_era: RewardEra,
) {
let max_history: RewardEra = <T as Config>::ProviderBoostHistoryLimit::get().into();
let starting_era = current_era - max_history - 1u32;
for i in starting_era..current_era {
assert_ok!(Capacity::<T>::upsert_boost_history(caller.into(), i, amount, true));
}
}

Expand Down Expand Up @@ -152,7 +159,7 @@ benchmarks! {

let current_era: RewardEra = (history_limit + 1u32).into();
CurrentEraInfo::<T>::set(RewardEraInfo{ era_index: current_era, started_at });
fill_reward_pool_chunks::<T>();
fill_reward_pool_chunks::<T>(current_era);
}: {
Capacity::<T>::start_new_reward_era_if_needed(current_block);
} verify {
Expand Down Expand Up @@ -232,12 +239,26 @@ benchmarks! {

}: _ (RawOrigin::Signed(caller.clone()), target, boost_amount)
verify {
assert!(StakingAccountLedger::<T>::contains_key(&caller));
assert!(StakingTargetLedger::<T>::contains_key(&caller, target));
assert!(CapacityLedger::<T>::contains_key(target));
assert_last_event::<T>(Event::<T>::ProviderBoosted {account: caller, amount: boost_amount, target, capacity}.into());
}

// TODO: vary the boost_history to get better weight estimates.
claim_staking_rewards {
let caller: T::AccountId = create_funded_account::<T>("account", SEED, 5u32);
let from_msa = 33;
let from_msa_amount: BalanceOf<T> = T::MinimumStakingAmount::get().saturating_add(31u32.into());
setup_provider_stake::<T>(&caller, &from_msa, from_msa_amount, false);
frame_system::Pallet::<T>::set_block_number(1002u32.into());
let current_era: RewardEra = 100u32;
set_era_and_reward_pool_at_block::<T>(current_era, 1001u32.into(), 1_000u32.into());
fill_reward_pool_chunks::<T>(current_era);
fill_boost_history::<T>(&caller, 1000u32.into(), current_era);
}: _ (RawOrigin::Signed(caller.clone()))
verify {
let expected_amount: BalanceOf<T> = 293u32.into();
assert_last_event::<T>(Event::<T>::ProviderBoostRewardClaimed {account: caller.clone(), reward_amount: expected_amount}.into());
}

impl_benchmark_test_suite!(Capacity,
tests::mock::new_test_ext(),
tests::mock::Test);
Expand Down
134 changes: 95 additions & 39 deletions pallets/capacity/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,30 @@ use sp_std::ops::Mul;
use frame_support::{
ensure,
traits::{
fungible::Inspect,
tokens::fungible::{Inspect as InspectFungible, InspectFreeze, Mutate, MutateFreeze},
Get, Hooks,
},
weights::{constants::RocksDbWeight, Weight},
};
use frame_system::pallet_prelude::BlockNumberFor;

use sp_runtime::{
traits::{CheckedAdd, CheckedDiv, One, Saturating, Zero},
ArithmeticError, BoundedVec, DispatchError, Perbill, Permill,
};

pub use common_primitives::{
capacity::{Nontransferable, Replenishable, TargetValidator},
capacity::*,
msa::MessageSourceId,
node::{AccountId, Balance, BlockNumber},
utils::wrap_binary_data,
};

use frame_system::pallet_prelude::*;

#[cfg(feature = "runtime-benchmarks")]
use common_primitives::benchmarks::RegisterProviderBenchmarkHelper;

pub use pallet::*;
pub use types::*;
pub use weights::*;
Expand All @@ -68,15 +73,11 @@ pub mod weights;
type BalanceOf<T> =
<<T as Config>::Currency as InspectFungible<<T as frame_system::Config>::AccountId>>::Balance;

use crate::StakingType::ProviderBoost;
use common_primitives::capacity::RewardEra;
use frame_system::pallet_prelude::*;

#[frame_support::pallet]
pub mod pallet {
use super::*;

use crate::StakingType::MaximumCapacity;
use crate::StakingType::*;
use common_primitives::capacity::RewardEra;
use frame_support::{
pallet_prelude::{StorageVersion, *},
Expand Down Expand Up @@ -350,6 +351,13 @@ pub mod pallet {
/// The Capacity amount issued to the target as a result of the stake.
capacity: BalanceOf<T>,
},
/// Provider Boost Token Rewards have been minted and transferred to the staking account.
ProviderBoostRewardClaimed {
/// The token account claiming and receiving the reward from ProviderBoost staking
account: T::AccountId,
/// The reward amount
reward_amount: BalanceOf<T>,
},
}

#[pallet::error]
Expand Down Expand Up @@ -402,6 +410,10 @@ pub mod pallet {
MaxRetargetsExceeded,
/// Tried to exceed bounds of a some Bounded collection
CollectionBoundExceeded,
/// This origin has nothing staked for ProviderBoost.
NotAProviderBoostAccount,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotAStakingAccount was getting a bit overloaded.

/// There are no unpaid rewards to claim from ProviderBoost staking.
NothingToClaim,
}

#[pallet::hooks]
Expand Down Expand Up @@ -601,6 +613,23 @@ pub mod pallet {

Ok(())
}

/// Claim all outstanding rewards earned from ProviderBoosting.
#[pallet::call_index(6)]
#[pallet::weight(T::WeightInfo::claim_staking_rewards())]
pub fn claim_staking_rewards(origin: OriginFor<T>) -> DispatchResult {
let staker = ensure_signed(origin)?;
ensure!(
ProviderBoostHistories::<T>::contains_key(staker.clone()),
Error::<T>::NotAProviderBoostAccount
);
let total_to_mint = Self::do_claim_rewards(&staker)?;
Self::deposit_event(Event::ProviderBoostRewardClaimed {
account: staker.clone(),
Copy link
Collaborator

@enddynayn enddynayn Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please check if the clone can be removed from here? It seems at first sight that we might not need to do a clone.

reward_amount: total_to_mint,
});
Ok(())
}
}
}

Expand Down Expand Up @@ -653,8 +682,8 @@ impl<T: Config> Pallet<T> {
amount: &BalanceOf<T>,
) -> Result<(StakingDetails<T>, BalanceOf<T>), DispatchError> {
let (mut staking_details, stakable_amount) =
Self::ensure_can_stake(staker, *target, *amount, ProviderBoost)?;
staking_details.staking_type = ProviderBoost;
Self::ensure_can_stake(staker, *target, *amount, StakingType::ProviderBoost)?;
staking_details.staking_type = StakingType::ProviderBoost;
Ok((staking_details, stakable_amount))
}

Expand Down Expand Up @@ -781,7 +810,7 @@ impl<T: Config> Pallet<T> {
Self::set_staking_account(unstaker, &staking_account);

let staking_type = staking_account.staking_type;
if staking_type == ProviderBoost {
if staking_type == StakingType::ProviderBoost {
let era = Self::get_current_era().era_index;
Self::upsert_boost_history(&unstaker, era, actual_unstaked_amount, false)?;
let reward_pool_total = CurrentEraProviderBoostTotal::<T>::get();
Expand Down Expand Up @@ -1024,37 +1053,42 @@ impl<T: Config> Pallet<T> {
Some(provider_boost_history) => {
match provider_boost_history.count() {
0usize => false,
// they staked before the current era, so they have unclaimed rewards.
1usize => provider_boost_history.get_entry_for_era(&current_era).is_none(),
1usize => {
Copy link
Collaborator

@enddynayn enddynayn Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing a match statement makes the code a bit more readable:

pub(crate) fn has_unclaimed_rewards(account: &T::AccountId) -> bool {
    let current_era = Self::get_current_era().era_index;

    if let Some(provider_boost_history) = Self::get_staking_history_for(account) {
        let entry_count = provider_boost_history.count();
        
        match entry_count {
            0 =>  false, 
            1 => {
                let previous_era = current_era.saturating_sub(1);
                provider_boost_history.get_entry_for_era(&previous_era).is_none() &&
                provider_boost_history.get_entry_for_era(&current_era).is_none()
           },
            _ => true
        }
    } else {
       false
    }
}

// if there is just one era entry and:
// it's for the previous era, it means we've already paid out rewards for that era, or they just staked in the last era.
// of if it's for the current era, they only just started staking.
provider_boost_history
.get_entry_for_era(&current_era.saturating_sub(1u32.into()))
.is_none() && provider_boost_history
.get_entry_for_era(&current_era)
.is_none()
},
_ => true,
}
},
None => false,
} // 1r
}

// this could be up to 35 reads.
#[allow(unused)]
pub(crate) fn list_unclaimed_rewards(
/// Get all unclaimed rewards information for each eligible Reward Era
pub fn list_unclaimed_rewards(
account: &T::AccountId,
) -> Result<BoundedVec<UnclaimedRewardInfo<T>, T::ProviderBoostHistoryLimit>, DispatchError> {
let mut unclaimed_rewards: BoundedVec<
UnclaimedRewardInfo<T>,
) -> Result<
BoundedVec<
UnclaimedRewardInfo<BalanceOf<T>, BlockNumberFor<T>>,
T::ProviderBoostHistoryLimit,
> = BoundedVec::new();

>,
DispatchError,
> {
if !Self::has_unclaimed_rewards(account) {
// 2r
return Ok(unclaimed_rewards);
return Ok(BoundedVec::new());
}

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

let current_era_info = Self::get_current_era(); // cached read, ditto
let max_history: u32 = T::ProviderBoostHistoryLimit::get(); // 1r
let era_length: u32 = T::EraLength::get(); // 1r length in blocks
let chunk_length: u32 = T::RewardPoolChunkLength::get();
let max_history: u32 = T::ProviderBoostHistoryLimit::get();

let mut reward_era = current_era_info.era_index.saturating_sub((max_history).into());
let end_era = current_era_info.era_index.saturating_sub(One::one());
Expand All @@ -1063,6 +1097,10 @@ impl<T: Config> Pallet<T> {
let mut previous_amount: BalanceOf<T> =
staking_history.get_amount_staked_for_era(&(reward_era.saturating_sub(1u32.into())));

let mut unclaimed_rewards: BoundedVec<
UnclaimedRewardInfo<BalanceOf<T>, BlockNumberFor<T>>,
T::ProviderBoostHistoryLimit,
> = BoundedVec::new();
while reward_era.le(&end_era) {
let staked_amount = staking_history.get_amount_staked_for_era(&reward_era);
if !staked_amount.is_zero() {
Expand All @@ -1084,6 +1122,7 @@ impl<T: Config> Pallet<T> {
.try_push(UnclaimedRewardInfo {
reward_era,
expires_at_block,
staked_amount,
eligible_amount,
earned_amount,
})
Expand Down Expand Up @@ -1134,10 +1173,12 @@ impl<T: Config> Pallet<T> {
/// Example with history limit of 6 and chunk length 3:
/// - Arrange the chunks 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]`
/// - `[0,1,2],[3,4,5],[6,7,8],[]`
/// Note Chunks stored = (History Length / Chunk size) + 1
/// - The second step is which chunk to add to:
/// - Divide the cycle by the chunk length and take the floor
/// - Floor(5 / 3) = 1
/// Chunk Index = Floor((era % (History Length + chunk size)) / chunk size)
pub(crate) fn get_chunk_index_for_era(era: RewardEra) -> u32 {
let history_limit: u32 = T::ProviderBoostHistoryLimit::get();
let chunk_len = T::RewardPoolChunkLength::get();
Expand All @@ -1150,11 +1191,6 @@ impl<T: Config> Pallet<T> {
}

// This is where the reward pool gets updated.
// 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: RewardEra, boost_total: BalanceOf<T>) {
// Current era is this era
let chunk_idx: u32 = Self::get_chunk_index_for_era(era);
Expand All @@ -1172,6 +1208,33 @@ impl<T: Config> Pallet<T> {
}
ProviderBoostRewardPools::<T>::set(chunk_idx, Some(new_chunk)); // 1w
}
fn do_claim_rewards(staker: &T::AccountId) -> Result<BalanceOf<T>, DispatchError> {
let rewards = Self::list_unclaimed_rewards(&staker)?;
ensure!(!rewards.len().is_zero(), Error::<T>::NothingToClaim);
let zero_balance: BalanceOf<T> = 0u32.into();
let total_to_mint: BalanceOf<T> = rewards
.iter()
.fold(zero_balance, |acc, reward_info| acc.saturating_add(reward_info.earned_amount))
.into();
ensure!(total_to_mint.gt(&Zero::zero()), Error::<T>::NothingToClaim);
let _minted_unused = T::Currency::mint_into(&staker, total_to_mint)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need let _minted_unused if we are not using it?


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;
// 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!(
new_history
.add_era_balance(&current_era.saturating_sub(1u32.into()), &last_staked_amount)
.is_some(),
Error::<T>::CollectionBoundExceeded
);
ProviderBoostHistories::<T>::set(staker, Some(new_history));

Ok(total_to_mint)
}
}

/// Nontransferable functions are intended for capacity spend and recharge.
Expand Down Expand Up @@ -1261,13 +1324,6 @@ impl<T: Config> ProviderBoostRewardsProvider<T> for Pallet<T> {
T::RewardPoolEachEra::get()
}

// TODO: implement or pull in list_unclaimed_rewards fn
fn staking_reward_totals(
_account_id: Self::AccountId,
) -> Result<BoundedVec<UnclaimedRewardInfo<T>, T::ProviderBoostHistoryLimit>, DispatchError> {
Ok(BoundedVec::new())
}

/// Calculate the reward for a single era. We don't care about the era number,
/// just the values.
fn era_staking_reward(
Expand Down
Loading