Improve backer rewards formula (#241)

* Improve rewards calculation for backers

* Update spec_version to 31

* Add a test to check rewards amounts

---------

Co-authored-by: Oleh Mell <olehmell@users.noreply.github.com>
Co-authored-by: Alex Siman <siman@users.noreply.github.com>

pallets/creator-staking/src/functions.rs

@@ -205,20 +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;
-        let combined_backers_reward_share = creator_proportional_stake * era_info.rewards.backers;
-
-        (creator_reward_share, combined_backers_reward_share)
+        creator_reward_ratio * era_info.rewards.creators
     }
 
     /// This utility function converts the PalletId specified in `Config` into an account ID.
@@ -363,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(reward_and_stake) = Self::general_era_info(era) {
-            let (_, combined_backers_reward_share) =
-                Self::distributed_rewards_between_creator_and_backers(creator_stake_info, &reward_and_stake);
-            Perbill::from_rational(staked, creator_stake_info.total_staked) * combined_backers_reward_share
+        if let Some(era_info) = Self::general_era_info(era) {
+            Perbill::from_rational(staked, era_info.staked) * era_info.rewards.backers
         } else {
             Zero::zero()
         }
@@ -429,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);
 
@@ -493,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);
                 }
             }
         }

pallets/creator-staking/src/lib.rs

@@ -612,14 +612,12 @@ pub mod pallet {
             let current_era = Self::current_era();
             ensure!(era_to_claim < current_era, Error::<T>::CannotClaimInFutureEra);
 
-            let staking_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(&staking_info, &reward_and_stake);
+            // TODO: move to separate function
             let backer_reward =
-                Perbill::from_rational(backer_staked, staking_info.total_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(
@@ -687,7 +685,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,
             );

pallets/creator-staking/src/tests/testing_utils.rs

@@ -15,12 +15,12 @@ use crate::tests::tests::Rewards;
 
 /// Helper struct used to store information relevant to era/creator/backer combination.
 pub(super) struct MemorySnapshot {
-    era_info: EraInfo<Balance>,
-    creator_info: CreatorInfo<AccountId>,
-    backer_stakes: StakesInfo<Balance, MaxEraStakeItems>,
-    creator_stakes_info: CreatorStakeInfo<Balance>,
-    free_balance: Balance,
-    backer_locks: BackerLocks<Balance, MaxUnbondingChunks>,
+    pub(super) era_info: EraInfo<Balance>,
+    pub(super) creator_info: CreatorInfo<AccountId>,
+    pub(super) backer_stakes: StakesInfo<Balance, MaxEraStakeItems>,
+    pub(super) creator_stakes_info: CreatorStakeInfo<Balance>,
+    pub(super) free_balance: Balance,
+    pub(super) backer_locks: BackerLocks<Balance, MaxUnbondingChunks>,
 }
 
 impl MemorySnapshot {
@@ -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,
     );
@@ -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
         );
 
@@ -584,18 +583,18 @@ 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
     );
 
     assert!(final_state.creator_stakes_info.rewards_claimed);
 
-    // Just in case dev is also a backer - this shouldn't cause any change in StakesInfo or BackerLocksByAccount
+    // Just in case creator is also a backer - this shouldn't cause any change in StakesInfo or BackerLocksByAccount
     assert_eq!(init_state.backer_stakes.stakes, final_state.backer_stakes.stakes);
     assert_eq!(init_state.backer_locks.total_locked, final_state.backer_locks.total_locked);
     assert_eq!(init_state.backer_locks.unbonding_info.vec(), final_state.backer_locks.unbonding_info.vec());

pallets/creator-staking/src/tests/tests.rs

@@ -1628,6 +1628,56 @@ fn claim_creator_with_zero_stake_periods_is_ok() {
     })
 }
 
+#[test]
+fn rewards_are_independent_of_total_staked_amount_for_creators() {
+    ExternalityBuilder::build().execute_with(|| {
+        initialize_first_block();
+
+        let start_era = CreatorStaking::current_era();
+        let registration_era = start_era + 1;
+        let staking_era = start_era + 2;
+        let claiming_era = start_era + 3;
+
+        let stakeholder = 10;
+        let first_creator_id = 1;
+        let second_creator_id = 2;
+
+        let dummy_backer_id = 1;
+        let backer_id = 2;
+
+        let stake_value = 100;
+
+        // Register creators
+        assert_register(stakeholder, first_creator_id);
+        assert_register(stakeholder, second_creator_id);
+        advance_to_era(registration_era);
+
+        // Make creators have different total stakes
+        assert_stake(dummy_backer_id, first_creator_id, 10);
+        assert_stake(dummy_backer_id, second_creator_id, 10_000);
+        advance_to_era(staking_era);
+
+        // Stake some tokens (stake amount not to change total staked) for both creators
+        assert_stake(backer_id, first_creator_id, stake_value);
+        assert_stake(backer_id, second_creator_id, stake_value);
+        advance_to_era(claiming_era);
+
+        // Claim rewards for both creators
+        let initial_backer_balance = Balances::free_balance(&backer_id);
+
+        assert_claim_backer(backer_id, first_creator_id, false);
+        let backer_reward_from_first_creator =
+            Balances::free_balance(&backer_id) - initial_backer_balance;
+
+        assert_claim_backer(backer_id, second_creator_id, false);
+        let backer_reward_from_second_creator =
+            Balances::free_balance(&backer_id) - backer_reward_from_first_creator - initial_backer_balance;
+
+        // Actual rewards should be equal since total staked amount doesn't affect reward
+        assert_eq!(backer_reward_from_first_creator, backer_reward_from_second_creator);
+    })
+}
+
 #[test]
 fn maintenance_mode_is_ok() {
     ExternalityBuilder::build().execute_with(|| {
@@ -1725,14 +1775,14 @@ fn maintenance_mode_no_change() {
 }
 
 #[test]
-fn distributed_rewards_between_creator_and_backers_util() {
+fn calculate_creator_reward_is_ok() {
     let base_backers_reward = 7 * 11 * 13 * 17;
     let base_creators_reward = 19 * 23 * 31;
     let staked_on_creator = 123456;
     let total_staked = staked_on_creator * 3;
 
     // Prepare structs
-    let staking_points = CreatorStakeInfo::<Balance> {
+    let creator_info = CreatorStakeInfo::<Balance> {
         total_staked: staked_on_creator,
         backers_count: 10,
         rewards_claimed: false,
@@ -1746,19 +1796,13 @@ 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 actual_creator_reward =
+        CreatorStaking::calculate_creator_reward(&creator_info, &era_info);
 
     let creator_stake_ratio = Perbill::from_rational(staked_on_creator, total_staked);
-    let calculated_backers_reward = creator_stake_ratio * base_backers_reward;
-    let calculated_dev_reward = creator_stake_ratio * base_creators_reward;
-    assert_eq!(calculated_dev_reward, creator_reward_share);
-    assert_eq!(calculated_backers_reward, combined_backers_reward_share);
-
-    assert_eq!(
-        calculated_backers_reward + calculated_dev_reward,
-        creator_reward_share + combined_backers_reward_share
-    );
+    let expected_creator_reward = creator_stake_ratio * base_creators_reward;
+
+    assert_eq!(expected_creator_reward, actual_creator_reward);
 }
 
 #[test]

runtime/src/lib.rs

@@ -176,7 +176,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("subsocial-parachain"),
 	impl_name: create_runtime_str!("subsocial-parachain"),
 	authoring_version: 1,
-	spec_version: 30,
+	spec_version: 31,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 5,