SIMD-0118: fix total_rewards for recalculation (#2780)

* Add new feature key

* Wrap existing code with new feature

* Extend test harness

* Make test fail

* Populate EpochRewards::total_rewards from PointValue

* Remove superfluous struct field

* Fixup tests

Author: CriesofCarrots

programs/bpf_loader/src/syscalls/mod.rs

@@ -42,7 +42,8 @@ use {
             enable_big_mod_exp_syscall, enable_get_epoch_stake_syscall,
             enable_partitioned_epoch_reward, enable_poseidon_syscall,
             error_on_syscall_bpf_function_hash_collisions, get_sysvar_syscall_enabled,
-            last_restart_slot_sysvar, reject_callx_r10, remaining_compute_units_syscall_enabled,
+            last_restart_slot_sysvar, partitioned_epoch_rewards_superfeature, reject_callx_r10,
+            remaining_compute_units_syscall_enabled,
         },
         hash::{Hash, Hasher},
         instruction::{AccountMeta, InstructionError, ProcessedSiblingInstruction},
@@ -273,8 +274,9 @@ pub fn create_program_runtime_environment_v1<'a>(
     let blake3_syscall_enabled = feature_set.is_active(&blake3_syscall_enabled::id());
     let curve25519_syscall_enabled = feature_set.is_active(&curve25519_syscall_enabled::id());
     let disable_fees_sysvar = feature_set.is_active(&disable_fees_sysvar::id());
-    let epoch_rewards_syscall_enabled =
-        feature_set.is_active(&enable_partitioned_epoch_reward::id());
+    let epoch_rewards_syscall_enabled = feature_set
+        .is_active(&enable_partitioned_epoch_reward::id())
+        || feature_set.is_active(&partitioned_epoch_rewards_superfeature::id());
     let disable_deploy_of_alloc_free_syscall = reject_deployment_of_broken_elfs
         && feature_set.is_active(&disable_deploy_of_alloc_free_syscall::id());
     let last_restart_slot_syscall_enabled = feature_set.is_active(&last_restart_slot_sysvar::id());

programs/stake/src/stake_instruction.rs

@@ -66,8 +66,8 @@ declare_process_instruction!(Entrypoint, DEFAULT_COMPUTE_UNITS, |invoke_context|
     };
 
     // The EpochRewards sysvar only exists after the
-    // enable_partitioned_epoch_reward feature is activated. If it exists, check
-    // the `active` field
+    // partitioned_epoch_rewards_superfeature feature is activated. If it
+    // exists, check the `active` field
     let epoch_rewards_active = invoke_context
         .get_sysvar_cache()
         .get_epoch_rewards()

rpc/src/rpc.rs

@@ -610,12 +610,20 @@ impl JsonRpcRequestProcessor {
         // epoch
         let bank = self.get_bank_with_config(context_config)?;
 
-        // DO NOT CLEAN UP with feature_set::enable_partitioned_epoch_reward
+        // DO NOT CLEAN UP with feature_set::partitioned_epoch_rewards_superfeature
         // This logic needs to be retained indefinitely to support historical
         // rewards before and after feature activation.
         let partitioned_epoch_reward_enabled_slot = bank
             .feature_set
-            .activated_slot(&feature_set::enable_partitioned_epoch_reward::id());
+            .activated_slot(&feature_set::partitioned_epoch_rewards_superfeature::id())
+            .or_else(|| {
+                // The order of these checks should not matter, since we will
+                // not ever have both features active on a live cluster. This
+                // check can be removed with
+                // feature_set::enable_partitioned_epoch_reward
+                bank.feature_set
+                    .activated_slot(&feature_set::enable_partitioned_epoch_reward::id())
+            });
         let partitioned_epoch_reward_enabled = partitioned_epoch_reward_enabled_slot
             .map(|slot| slot <= first_confirmed_block_in_epoch)
             .unwrap_or(false);

runtime/src/bank/partitioned_epoch_rewards/calculation.rs

@@ -50,7 +50,7 @@ impl Bank {
         let CalculateRewardsAndDistributeVoteRewardsResult {
             total_rewards,
             distributed_rewards,
-            total_points,
+            point_value,
             stake_rewards_by_partition,
         } = self.calculate_rewards_and_distribute_vote_rewards(
             parent_epoch,
@@ -80,7 +80,7 @@ impl Bank {
             distributed_rewards,
             distribution_starting_block_height,
             num_partitions,
-            total_points,
+            point_value,
         );
 
         datapoint_info!(
@@ -105,12 +105,11 @@ impl Bank {
             vote_account_rewards,
             stake_rewards_by_partition,
             old_vote_balance_and_staked,
-            validator_rewards,
             validator_rate,
             foundation_rate,
             prev_epoch_duration_in_years,
             capitalization,
-            total_points,
+            point_value,
         } = self.calculate_rewards_for_partitioning(
             prev_epoch,
             reward_calc_tracer,
@@ -136,11 +135,11 @@ impl Bank {
         self.assert_validator_rewards_paid(validator_rewards_paid);
 
         // verify that we didn't pay any more than we expected to
-        assert!(validator_rewards >= validator_rewards_paid + total_stake_rewards_lamports);
+        assert!(point_value.rewards >= validator_rewards_paid + total_stake_rewards_lamports);
 
         info!(
             "distributed vote rewards: {} out of {}, remaining {}",
-            validator_rewards_paid, validator_rewards, total_stake_rewards_lamports
+            validator_rewards_paid, point_value.rewards, total_stake_rewards_lamports
         );
 
         let (num_stake_accounts, num_vote_accounts) = {
@@ -179,7 +178,7 @@ impl Bank {
         CalculateRewardsAndDistributeVoteRewardsResult {
             total_rewards: validator_rewards_paid + total_stake_rewards_lamports,
             distributed_rewards: validator_rewards_paid,
-            total_points,
+            point_value,
             stake_rewards_by_partition,
         }
     }
@@ -231,7 +230,7 @@ impl Bank {
         let CalculateValidatorRewardsResult {
             vote_rewards_accounts: vote_account_rewards,
             stake_reward_calculation: mut stake_rewards,
-            total_points,
+            point_value,
         } = self
             .calculate_validator_rewards(
                 prev_epoch,
@@ -260,12 +259,11 @@ impl Bank {
                 total_stake_rewards_lamports: stake_rewards.total_stake_rewards_lamports,
             },
             old_vote_balance_and_staked,
-            validator_rewards,
             validator_rate,
             foundation_rate,
             prev_epoch_duration_in_years,
             capitalization,
-            total_points,
+            point_value,
         }
     }
 
@@ -288,20 +286,19 @@ impl Bank {
             metrics,
         )
         .map(|point_value| {
-            let total_points = point_value.points;
             let (vote_rewards_accounts, stake_reward_calculation) = self
                 .calculate_stake_vote_rewards(
                     &reward_calculate_param,
                     rewarded_epoch,
-                    point_value,
+                    point_value.clone(),
                     thread_pool,
                     reward_calc_tracer,
                     metrics,
                 );
             CalculateValidatorRewardsResult {
                 vote_rewards_accounts,
                 stake_reward_calculation,
-                total_points,
+                point_value,
             }
         })
     }
@@ -601,7 +598,8 @@ mod tests {
                 null_tracer,
                 partitioned_epoch_rewards::{
                     tests::{
-                        create_default_reward_bank, create_reward_bank, RewardBank, SLOTS_PER_EPOCH,
+                        create_default_reward_bank, create_reward_bank,
+                        create_reward_bank_with_specific_stakes, RewardBank, SLOTS_PER_EPOCH,
                     },
                     EpochRewardStatus, StartBlockHeightAndRewards,
                 },
@@ -988,11 +986,14 @@ mod tests {
 
     #[test]
     fn test_recalculate_partitioned_rewards() {
-        let expected_num_delegations = 4;
+        let expected_num_delegations = 3;
         let num_rewards_per_block = 2;
         // Distribute 4 rewards over 2 blocks
-        let RewardBank { bank, .. } = create_reward_bank(
-            expected_num_delegations,
+        let mut stakes = vec![2_000_000_000; expected_num_delegations];
+        // Add stake large enough to be affected by total-rewards discrepancy
+        stakes.push(40_000_000_000);
+        let RewardBank { bank, .. } = create_reward_bank_with_specific_stakes(
+            stakes,
             num_rewards_per_block,
             SLOTS_PER_EPOCH - 1,
         );
@@ -1012,6 +1013,7 @@ mod tests {
                     stake_rewards_by_partition: expected_stake_rewards,
                     ..
                 },
+            point_value,
             ..
         } = bank.calculate_rewards_for_partitioning(
             rewarded_epoch,
@@ -1035,6 +1037,9 @@ mod tests {
         assert_eq!(expected_stake_rewards.len(), recalculated_rewards.len());
         compare_stake_rewards(&expected_stake_rewards, recalculated_rewards);
 
+        let sysvar = bank.get_epoch_rewards_sysvar();
+        assert_eq!(point_value.rewards, sysvar.total_rewards);
+
         // Advance to first distribution slot
         let mut bank =
             Bank::new_from_parent(Arc::new(bank), &Pubkey::default(), SLOTS_PER_EPOCH + 1);

runtime/src/bank/partitioned_epoch_rewards/distribution.rs

@@ -255,7 +255,7 @@ mod tests {
             },
             sysvar,
         },
-        solana_stake_program::stake_state,
+        solana_stake_program::{points::PointValue, stake_state},
         solana_vote_program::vote_state,
     };
 
@@ -349,13 +349,22 @@ mod tests {
             create_genesis_config(1_000_000 * LAMPORTS_PER_SOL);
         genesis_config.epoch_schedule = EpochSchedule::custom(432000, 432000, false);
         let mut bank = Bank::new_for_tests(&genesis_config);
-        bank.activate_feature(&feature_set::enable_partitioned_epoch_reward::id());
+        bank.activate_feature(&feature_set::partitioned_epoch_rewards_superfeature::id());
 
         // Set up epoch_rewards sysvar with rewards with 1e9 lamports to distribute.
         let total_rewards = 1_000_000_000;
         let num_partitions = 2; // num_partitions is arbitrary and unimportant for this test
         let total_points = (total_rewards * 42) as u128; // total_points is arbitrary for the purposes of this test
-        bank.create_epoch_rewards_sysvar(total_rewards, 0, 42, num_partitions, total_points);
+        bank.create_epoch_rewards_sysvar(
+            total_rewards,
+            0,
+            42,
+            num_partitions,
+            PointValue {
+                rewards: total_rewards,
+                points: total_points,
+            },
+        );
         let pre_epoch_rewards_account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap();
         let expected_balance =
             bank.get_minimum_balance_for_rent_exemption(pre_epoch_rewards_account.data().len());

runtime/src/bank/partitioned_epoch_rewards/mod.rs

@@ -18,6 +18,7 @@ use {
         reward_info::RewardInfo,
         stake::state::{Delegation, Stake, StakeStateV2},
     },
+    solana_stake_program::points::PointValue,
     solana_vote::vote_account::VoteAccounts,
     std::sync::Arc,
 };
@@ -95,11 +96,24 @@ struct StakeRewardCalculation {
     total_stake_rewards_lamports: u64,
 }
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 struct CalculateValidatorRewardsResult {
     vote_rewards_accounts: VoteRewardsAccounts,
     stake_reward_calculation: StakeRewardCalculation,
-    total_points: u128,
+    point_value: PointValue,
+}
+
+impl Default for CalculateValidatorRewardsResult {
+    fn default() -> Self {
+        Self {
+            vote_rewards_accounts: VoteRewardsAccounts::default(),
+            stake_reward_calculation: StakeRewardCalculation::default(),
+            point_value: PointValue {
+                points: 0,
+                rewards: 0,
+            },
+        }
+    }
 }
 
 /// hold reward calc info to avoid recalculation across functions
@@ -116,12 +130,11 @@ pub(super) struct PartitionedRewardsCalculation {
     pub(super) vote_account_rewards: VoteRewardsAccounts,
     pub(super) stake_rewards_by_partition: StakeRewardCalculationPartitioned,
     pub(super) old_vote_balance_and_staked: u64,
-    pub(super) validator_rewards: u64,
     pub(super) validator_rate: f64,
     pub(super) foundation_rate: f64,
     pub(super) prev_epoch_duration_in_years: f64,
     pub(super) capitalization: u64,
-    total_points: u128,
+    point_value: PointValue,
 }
 
 /// result of calculating the stake rewards at beginning of new epoch
@@ -133,14 +146,16 @@ pub(super) struct StakeRewardCalculationPartitioned {
 }
 
 pub(super) struct CalculateRewardsAndDistributeVoteRewardsResult {
-    /// total rewards for the epoch (including both vote rewards and stake rewards)
+    /// total rewards to be distributed in the epoch (including both vote
+    /// rewards and stake rewards)
     pub(super) total_rewards: u64,
     /// distributed vote rewards
     pub(super) distributed_rewards: u64,
-    /// total rewards points calculated for the current epoch, where points
+    /// total rewards and points calculated for the current epoch, where points
     /// equals the sum of (delegated stake * credits observed) for all
-    /// delegations
-    pub(super) total_points: u128,
+    /// delegations and rewards are the lamports to split across all stake and
+    /// vote accounts
+    pub(super) point_value: PointValue,
     /// stake rewards that still need to be distributed, grouped by partition
     pub(super) stake_rewards_by_partition: Vec<PartitionedStakeRewards>,
 }
@@ -180,6 +195,9 @@ impl Bank {
     pub(super) fn is_partitioned_rewards_feature_enabled(&self) -> bool {
         self.feature_set
             .is_active(&feature_set::enable_partitioned_epoch_reward::id())
+            || self
+                .feature_set
+                .is_active(&feature_set::partitioned_epoch_rewards_superfeature::id())
     }
 
     pub(crate) fn set_epoch_reward_status_active(
@@ -347,17 +365,25 @@ mod tests {
         stake_account_stores_per_block: u64,
         advance_num_slots: u64,
     ) -> RewardBank {
-        let validator_keypairs = (0..expected_num_delegations)
+        create_reward_bank_with_specific_stakes(
+            vec![2_000_000_000; expected_num_delegations],
+            stake_account_stores_per_block,
+            advance_num_slots,
+        )
+    }
+
+    pub(super) fn create_reward_bank_with_specific_stakes(
+        stakes: Vec<u64>,
+        stake_account_stores_per_block: u64,
+        advance_num_slots: u64,
+    ) -> RewardBank {
+        let validator_keypairs = (0..stakes.len())
             .map(|_| ValidatorVoteKeypairs::new_rand())
             .collect::<Vec<_>>();
 
         let GenesisConfigInfo {
             mut genesis_config, ..
-        } = create_genesis_config_with_vote_accounts(
-            1_000_000_000,
-            &validator_keypairs,
-            vec![2_000_000_000; expected_num_delegations],
-        );
+        } = create_genesis_config_with_vote_accounts(1_000_000_000, &validator_keypairs, stakes);
         genesis_config.epoch_schedule = EpochSchedule::new(SLOTS_PER_EPOCH);
 
         let mut accounts_db_config: AccountsDbConfig = ACCOUNTS_DB_CONFIG_FOR_TESTING.clone();
@@ -456,7 +482,7 @@ mod tests {
 
         let mut bank = Bank::new_for_tests(&genesis_config);
         assert!(!bank.is_partitioned_rewards_feature_enabled());
-        bank.activate_feature(&feature_set::enable_partitioned_epoch_reward::id());
+        bank.activate_feature(&feature_set::partitioned_epoch_rewards_superfeature::id());
         assert!(bank.is_partitioned_rewards_feature_enabled());
     }
 
@@ -950,6 +976,9 @@ mod tests {
         genesis_config
             .accounts
             .remove(&feature_set::enable_partitioned_epoch_reward::id());
+        genesis_config
+            .accounts
+            .remove(&feature_set::partitioned_epoch_rewards_superfeature::id());
 
         let bank = Bank::new_for_tests(&genesis_config);