Skip to content

Commit

Permalink
Provide code refactoring after review
Browse files Browse the repository at this point in the history
  • Loading branch information
F3Joule committed Nov 8, 2023
1 parent d0f4392 commit c23b4d1
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 52 deletions.
29 changes: 11 additions & 18 deletions pallets/creator-staking/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,19 +205,15 @@ impl<T: Config> Pallet<T> {
}
}

/// Calculate the reward distribution between a creator and everyone staking towards that creator.
///
/// Returns (creator's reward, backers' combined reward)
pub(crate) fn distributed_rewards_between_creator_and_backers(
/// Calculate the reward for a creator in a specific era.
pub(crate) fn calculate_creator_reward(
creator_info: &CreatorStakeInfo<BalanceOf<T>>,
era_info: &EraInfo<BalanceOf<T>>,
) -> (BalanceOf<T>, BalanceOf<T>) {
let creator_proportional_stake =
) -> BalanceOf<T> {
let creator_reward_ratio =
Perbill::from_rational(creator_info.total_staked, era_info.staked);

let creator_reward_share = creator_proportional_stake * era_info.rewards.creators;

(creator_reward_share, era_info.rewards.backers)
creator_reward_ratio * era_info.rewards.creators
}

/// This utility function converts the PalletId specified in `Config` into an account ID.
Expand Down Expand Up @@ -362,14 +358,11 @@ impl<T: Config> Pallet<T> {
}

pub(crate) fn calculate_reward_for_backer_in_era(
creator_stake_info: &CreatorStakeInfo<BalanceOf<T>>,
staked: BalanceOf<T>,
era: EraIndex,
) -> BalanceOf<T> {
if let Some(era_info) = Self::general_era_info(era) {
let (_, combined_backers_reward_share) =
Self::distributed_rewards_between_creator_and_backers(creator_stake_info, &era_info);
Perbill::from_rational(staked, era_info.staked) * combined_backers_reward_share
Perbill::from_rational(staked, era_info.staked) * era_info.rewards.backers
} else {
Zero::zero()
}
Expand Down Expand Up @@ -428,8 +421,8 @@ impl<T: Config> Pallet<T> {
break;
}

let backer_era_reward = |creator_stake_info| {
Self::calculate_reward_for_backer_in_era(&creator_stake_info, staked, era)
let backer_era_reward = |_| {
Self::calculate_reward_for_backer_in_era(staked, era)
};
let creator_stake_info = Self::creator_stake_info(creator_id, era);

Expand Down Expand Up @@ -492,10 +485,10 @@ impl<T: Config> Pallet<T> {
for (era, stake_info) in CreatorStakeInfoByEra::<T>::iter_prefix(creator_id) {
if !stake_info.rewards_claimed {
if let Some(era_info) = Self::general_era_info(era) {
let (creator_reward_share, _) =
Self::distributed_rewards_between_creator_and_backers(&stake_info, &era_info);
let creator_reward =
Self::calculate_creator_reward(&stake_info, &era_info);
estimated_rewards = estimated_rewards
.saturating_add(creator_reward_share);
.saturating_add(creator_reward);
}
}
}
Expand Down
7 changes: 2 additions & 5 deletions pallets/creator-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -612,14 +612,11 @@ pub mod pallet {
let current_era = Self::current_era();
ensure!(era_to_claim < current_era, Error::<T>::CannotClaimInFutureEra);

let creator_stake_info = Self::creator_stake_info(creator_id, era_to_claim).unwrap_or_default();
let reward_and_stake =
Self::general_era_info(era_to_claim).ok_or(Error::<T>::EraNotFound)?;

let (_, combined_backers_reward_share) =
Self::distributed_rewards_between_creator_and_backers(&creator_stake_info, &reward_and_stake);
let backer_reward =
Perbill::from_rational(backer_staked, reward_and_stake.staked) * combined_backers_reward_share;
Perbill::from_rational(backer_staked, reward_and_stake.staked) * reward_and_stake.rewards.backers;

// FIXME: we mustn't modify `backer_stakes` here!
let can_restake_reward = Self::ensure_can_restake_reward(
Expand Down Expand Up @@ -687,7 +684,7 @@ pub mod pallet {
Self::general_era_info(era).ok_or(Error::<T>::EraNotFound)?;

// Calculate the creator reward for this era.
let (creator_reward, _) = Self::distributed_rewards_between_creator_and_backers(
let creator_reward = Self::calculate_creator_reward(
&creator_stake_info,
&rewards_and_stakes,
);
Expand Down
9 changes: 4 additions & 5 deletions pallets/creator-staking/src/tests/testing_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,6 @@ pub(super) fn assert_claim_backer(claimer: AccountId, creator_id: SpaceId, resta
}

let calculated_reward = CreatorStaking::calculate_reward_for_backer_in_era(
&init_state_claim_era.creator_stakes_info,
staked,
claim_era,
);
Expand Down Expand Up @@ -572,8 +571,8 @@ pub(super) fn assert_claim_creator(creator_id: SpaceId, claim_era: EraIndex) {
}

// Calculate creator portion of the reward
let (creator_reward_share, _) =
CreatorStaking::distributed_rewards_between_creator_and_backers(
let creator_reward =
CreatorStaking::calculate_creator_reward(
&init_state.creator_stakes_info, &init_state.era_info
);

Expand All @@ -584,12 +583,12 @@ pub(super) fn assert_claim_creator(creator_id: SpaceId, claim_era: EraIndex) {
));
System::assert_last_event(mock::RuntimeEvent::CreatorStaking(Event::CreatorRewardsClaimed {
who: stakeholder.clone(),
amount: creator_reward_share,
amount: creator_reward,
}));

let final_state = MemorySnapshot::all(claim_era, creator_id, stakeholder);
assert_eq!(
init_state.free_balance + creator_reward_share,
init_state.free_balance + creator_reward,
final_state.free_balance
);

Expand Down
29 changes: 5 additions & 24 deletions pallets/creator-staking/src/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1662,25 +1662,6 @@ fn rewards_are_independent_of_total_staked_amount_for_creators() {
assert_stake(backer_id, second_creator_id, stake_value);
advance_to_era(claiming_era);

// Calculate expected reward
let first_creator_snapshot = MemorySnapshot::creator(claiming_era, first_creator_id);
let second_creator_snapshot = MemorySnapshot::creator(claiming_era, second_creator_id);

let expected_reward_for_first_creator = CreatorStaking::calculate_reward_for_backer_in_era(
&first_creator_snapshot.creator_stakes_info,
stake_value,
staking_era,
);

let expected_reward_for_second_creator = CreatorStaking::calculate_reward_for_backer_in_era(
&second_creator_snapshot.creator_stakes_info,
stake_value,
staking_era,
);

// Expected rewards should be equal since total staked amount doesn't affect reward
assert_eq!(expected_reward_for_first_creator, expected_reward_for_second_creator);

// Claim rewards for both creators
let initial_backer_balance = Balances::free_balance(&backer_id);

Expand Down Expand Up @@ -1814,18 +1795,18 @@ fn distributed_rewards_between_creator_and_backers_util() {
locked: total_staked,
};

let (creator_reward_share, combined_backers_reward_share) =
CreatorStaking::distributed_rewards_between_creator_and_backers(&staking_points, &era_info);
let creator_reward =
CreatorStaking::calculate_creator_reward(&staking_points, &era_info);

let creator_stake_ratio = Perbill::from_rational(staked_on_creator, total_staked);
let calculated_backers_reward = base_backers_reward;
let calculated_creator_reward = creator_stake_ratio * base_creators_reward;
assert_eq!(calculated_creator_reward, creator_reward_share);
assert_eq!(calculated_backers_reward, combined_backers_reward_share);
assert_eq!(calculated_creator_reward, creator_reward);
assert_eq!(calculated_backers_reward, era_info.rewards.backers);

assert_eq!(
calculated_backers_reward + calculated_creator_reward,
creator_reward_share + combined_backers_reward_share
creator_reward + era_info.rewards.backers
);
}

Expand Down

0 comments on commit c23b4d1

Please sign in to comment.