Persist EpochRewards sysvar (#572)

* Persist EpochRewards sysvar between reward intervals

* Adjust initial EpochRewards balance to ensure it is not debited out of existence

* Set EpochRewards::active = false at end of distribution

* Fix tests

* Extend test to 2 epochs, assert sysvar still exists

* Stop adjusting EpochRewards balance based on rewards

* Fix tests

* Review suggestions

program-test/tests/sysvar.rs

@@ -65,10 +65,11 @@ fn epoch_reward_sysvar_getter_process_instruction(
     // input[0] == 1 indicates the bank is in reward period.
     if input[0] == 0 {
         // epoch rewards sysvar should not exist for banks that are not in reward period
-        let epoch_rewards = EpochRewards::get();
-        assert!(epoch_rewards.is_err());
+        let epoch_rewards = EpochRewards::get()?;
+        assert!(!epoch_rewards.active);
     } else {
-        let _epoch_rewards = EpochRewards::get()?;
+        let epoch_rewards = EpochRewards::get()?;
+        assert!(epoch_rewards.active);
     }
 
     Ok(())

runtime/src/bank/partitioned_epoch_rewards/distribution.rs

@@ -45,7 +45,7 @@ impl Bank {
                 EpochRewardStatus::Active(_)
             ));
             self.epoch_reward_status = EpochRewardStatus::Inactive;
-            self.destroy_epoch_rewards_sysvar();
+            self.set_epoch_rewards_sysvar_to_inactive();
         }
     }
 
@@ -211,7 +211,10 @@ mod tests {
         let total_rewards = 1_000_000_000;
         bank.create_epoch_rewards_sysvar(total_rewards, 0, 42);
         let pre_epoch_rewards_account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap();
-        assert_eq!(pre_epoch_rewards_account.lamports(), total_rewards);
+        let expected_balance =
+            bank.get_minimum_balance_for_rent_exemption(pre_epoch_rewards_account.data().len());
+        // Expected balance is the sysvar rent-exempt balance
+        assert_eq!(pre_epoch_rewards_account.lamports(), expected_balance);
 
         // Set up a partition of rewards to distribute
         let expected_num = 100;
@@ -230,22 +233,18 @@ mod tests {
         bank.distribute_epoch_rewards_in_partition(&all_rewards, 0);
         let post_cap = bank.capitalization();
         let post_epoch_rewards_account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap();
-        let expected_epoch_rewards_sysvar_lamports_remaining =
-            total_rewards - rewards_to_distribute;
 
-        // Assert that epoch rewards sysvar lamports decreases by the distributed rewards
-        assert_eq!(
-            post_epoch_rewards_account.lamports(),
-            expected_epoch_rewards_sysvar_lamports_remaining
-        );
+        // Assert that epoch rewards sysvar lamports balance does not change
+        assert_eq!(post_epoch_rewards_account.lamports(), expected_balance);
 
         let epoch_rewards: sysvar::epoch_rewards::EpochRewards =
             from_account(&post_epoch_rewards_account).unwrap();
         assert_eq!(epoch_rewards.total_rewards, total_rewards);
         assert_eq!(epoch_rewards.distributed_rewards, rewards_to_distribute,);
 
-        // Assert that the bank total capital didn't change
-        assert_eq!(pre_cap, post_cap);
+        // Assert that the bank total capital changed by the amount of rewards
+        // distributed
+        assert_eq!(pre_cap + rewards_to_distribute, post_cap);
     }
 
     /// Test partitioned credits and reward history updates of epoch rewards do cover all the rewards

runtime/src/bank/partitioned_epoch_rewards/mod.rs

@@ -379,15 +379,19 @@ mod tests {
             .map(|_| ValidatorVoteKeypairs::new_rand())
             .collect::<Vec<_>>();
 
-        let GenesisConfigInfo { genesis_config, .. } = create_genesis_config_with_vote_accounts(
+        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],
         );
+        let slots_per_epoch = 32;
+        genesis_config.epoch_schedule = EpochSchedule::new(slots_per_epoch);
 
         let bank0 = Bank::new_for_tests(&genesis_config);
         let num_slots_in_epoch = bank0.get_slots_in_epoch(bank0.epoch());
-        assert_eq!(num_slots_in_epoch, 32);
+        assert_eq!(num_slots_in_epoch, slots_per_epoch);
 
         let mut previous_bank = Arc::new(Bank::new_from_parent(
             Arc::new(bank0),
@@ -396,7 +400,7 @@ mod tests {
         ));
 
         // simulate block progress
-        for slot in 2..=num_slots_in_epoch + 2 {
+        for slot in 2..=(2 * slots_per_epoch) + 2 {
             let pre_cap = previous_bank.capitalization();
             let curr_bank = Bank::new_from_parent(previous_bank, &Pubkey::default(), slot);
             let post_cap = curr_bank.capitalization();
@@ -425,32 +429,57 @@ mod tests {
                 curr_bank.store_account_and_update_capitalization(&vote_id, &vote_account);
             }
 
-            if slot == num_slots_in_epoch {
+            if slot % num_slots_in_epoch == 0 {
                 // This is the first block of epoch 1. Reward computation should happen in this block.
                 // assert reward compute status activated at epoch boundary
                 assert_matches!(
                     curr_bank.get_reward_interval(),
                     RewardInterval::InsideInterval
                 );
 
-                // cap should increase because of new epoch rewards
-                assert!(post_cap > pre_cap);
-            } else if slot == num_slots_in_epoch + 1 || slot == num_slots_in_epoch + 2 {
-                // 1. when curr_slot == num_slots_in_epoch + 1, the 2nd block of epoch 1, reward distribution should happen in this block.
-                // however, all stake rewards are paid at the this block therefore reward_status should have transitioned to inactive. And since
-                // rewards are transferred from epoch_rewards sysvar to stake accounts. The cap should stay the same.
-                // 2. when curr_slot == num_slots_in_epoch+2, the 3rd block of epoch 1. reward distribution should have already completed. Therefore,
-                // reward_status should stay inactive and cap should stay the same.
+                if slot == num_slots_in_epoch {
+                    // cap should increase because of new epoch rewards
+                    assert!(post_cap > pre_cap);
+                } else {
+                    assert_eq!(post_cap, pre_cap);
+                }
+            } else if slot == num_slots_in_epoch + 1 {
+                // 1. when curr_slot == num_slots_in_epoch + 1, the 2nd block of
+                // epoch 1, reward distribution should happen in this block.
+                // however, all stake rewards are paid at this block therefore
+                // reward_status should have transitioned to inactive. The cap
+                // should increase accordingly.
                 assert_matches!(
                     curr_bank.get_reward_interval(),
                     RewardInterval::OutsideInterval
                 );
 
-                assert_eq!(post_cap, pre_cap);
+                let account = curr_bank
+                    .get_account(&solana_sdk::sysvar::epoch_rewards::id())
+                    .unwrap();
+                let epoch_rewards: solana_sdk::sysvar::epoch_rewards::EpochRewards =
+                    solana_sdk::account::from_account(&account).unwrap();
+                assert_eq!(post_cap, pre_cap + epoch_rewards.distributed_rewards);
             } else {
+                // 2. when curr_slot == num_slots_in_epoch+2, the 3rd block of
+                // epoch 1 (or any other slot). reward distribution should have
+                // already completed. Therefore, reward_status should stay
+                // inactive and cap should stay the same.
+                assert_matches!(
+                    curr_bank.get_reward_interval(),
+                    RewardInterval::OutsideInterval
+                );
+
                 // slot is not in rewards, cap should not change
                 assert_eq!(post_cap, pre_cap);
             }
+            // EpochRewards sysvar is created in the first block of epoch 1.
+            // Ensure the sysvar persists thereafter.
+            if slot >= num_slots_in_epoch {
+                let epoch_rewards_lamports =
+                    curr_bank.get_balance(&solana_sdk::sysvar::epoch_rewards::id());
+                assert!(epoch_rewards_lamports > 0);
+            }
             previous_bank = Arc::new(curr_bank);
         }
     }
@@ -513,6 +542,14 @@ mod tests {
         // simulate block progress
         for slot in 2..=num_slots_in_epoch + 3 {
             let pre_cap = previous_bank.capitalization();
+
+            let pre_sysvar_account = previous_bank
+                .get_account(&solana_sdk::sysvar::epoch_rewards::id())
+                .unwrap_or_default();
+            let pre_epoch_rewards: solana_sdk::sysvar::epoch_rewards::EpochRewards =
+                solana_sdk::account::from_account(&pre_sysvar_account).unwrap_or_default();
+            let pre_distributed_rewards = pre_epoch_rewards.distributed_rewards;
+
             let curr_bank = Bank::new_from_parent(previous_bank, &Pubkey::default(), slot);
             let post_cap = curr_bank.capitalization();
 
@@ -551,27 +588,53 @@ mod tests {
                 // cap should increase because of new epoch rewards
                 assert!(post_cap > pre_cap);
             } else if slot == num_slots_in_epoch + 1 {
-                // When curr_slot == num_slots_in_epoch + 1, the 2nd block of epoch 1, reward distribution should happen in this block.
-                // however, since rewards are transferred from epoch_rewards sysvar to stake accounts. The cap should stay the same.
+                // When curr_slot == num_slots_in_epoch + 1, the 2nd block of
+                // epoch 1, reward distribution should happen in this block. The
+                // cap should increase accordingly.
                 assert_matches!(
                     curr_bank.get_reward_interval(),
                     RewardInterval::InsideInterval
                 );
 
-                assert_eq!(post_cap, pre_cap);
-            } else if slot == num_slots_in_epoch + 2 || slot == num_slots_in_epoch + 3 {
-                // 1. when curr_slot == num_slots_in_epoch + 2, the 3nd block of epoch 1, reward distribution should happen in this block.
-                // however, all stake rewards are paid at the this block therefore reward_status should have transitioned to inactive. And since
-                // rewards are transferred from epoch_rewards sysvar to stake accounts. The cap should stay the same.
-                // 2. when curr_slot == num_slots_in_epoch+2, the 3rd block of epoch 1. reward distribution should have already completed. Therefore,
-                // reward_status should stay inactive and cap should stay the same.
+                let account = curr_bank
+                    .get_account(&solana_sdk::sysvar::epoch_rewards::id())
+                    .unwrap();
+                let epoch_rewards: solana_sdk::sysvar::epoch_rewards::EpochRewards =
+                    solana_sdk::account::from_account(&account).unwrap();
+                assert_eq!(
+                    post_cap,
+                    pre_cap + epoch_rewards.distributed_rewards - pre_distributed_rewards
+                );
+            } else if slot == num_slots_in_epoch + 2 {
+                // When curr_slot == num_slots_in_epoch + 2, the 3nd block of
+                // epoch 1, reward distribution should happen in this block.
+                // however, all stake rewards are paid at the this block
+                // therefore reward_status should have transitioned to inactive.
+                // The cap should increase accordingly.
                 assert_matches!(
                     curr_bank.get_reward_interval(),
                     RewardInterval::OutsideInterval
                 );
 
-                assert_eq!(post_cap, pre_cap);
+                let account = curr_bank
+                    .get_account(&solana_sdk::sysvar::epoch_rewards::id())
+                    .unwrap();
+                let epoch_rewards: solana_sdk::sysvar::epoch_rewards::EpochRewards =
+                    solana_sdk::account::from_account(&account).unwrap();
+                assert_eq!(
+                    post_cap,
+                    pre_cap + epoch_rewards.distributed_rewards - pre_distributed_rewards
+                );
             } else {
+                // When curr_slot == num_slots_in_epoch + 3, the 4th block of
+                // epoch 1 (or any other slot). reward distribution should have
+                // already completed. Therefore, reward_status should stay
+                // inactive and cap should stay the same.
+                assert_matches!(
+                    curr_bank.get_reward_interval(),
+                    RewardInterval::OutsideInterval
+                );
+
                 // slot is not in rewards, cap should not change
                 assert_eq!(post_cap, pre_cap);
             }

runtime/src/bank/partitioned_epoch_rewards/sysvar.rs

@@ -33,6 +33,8 @@ impl Bank {
     ) {
         assert!(self.is_partitioned_rewards_code_enabled());
 
+        assert!(total_rewards >= distributed_rewards);
+
         let epoch_rewards = sysvar::epoch_rewards::EpochRewards {
             total_rewards,
             distributed_rewards,
@@ -42,13 +44,10 @@ impl Bank {
         };
 
         self.update_sysvar_account(&sysvar::epoch_rewards::id(), |account| {
-            let mut inherited_account_fields =
-                self.inherit_specially_retained_account_fields(account);
-
-            assert!(total_rewards >= distributed_rewards);
-            // set the account lamports to the undistributed rewards
-            inherited_account_fields.0 = total_rewards - distributed_rewards;
-            create_account(&epoch_rewards, inherited_account_fields)
+            create_account(
+                &epoch_rewards,
+                self.inherit_specially_retained_account_fields(account),
+            )
         });
 
         self.log_epoch_rewards_sysvar("create");
@@ -66,30 +65,37 @@ impl Bank {
         epoch_rewards.distribute(distributed);
 
         self.update_sysvar_account(&sysvar::epoch_rewards::id(), |account| {
-            let mut inherited_account_fields =
-                self.inherit_specially_retained_account_fields(account);
-
-            let lamports = inherited_account_fields.0;
-            assert!(lamports >= distributed);
-            inherited_account_fields.0 = lamports - distributed;
-            create_account(&epoch_rewards, inherited_account_fields)
+            create_account(
+                &epoch_rewards,
+                self.inherit_specially_retained_account_fields(account),
+            )
         });
 
         self.log_epoch_rewards_sysvar("update");
     }
 
-    pub(in crate::bank::partitioned_epoch_rewards) fn destroy_epoch_rewards_sysvar(&self) {
-        if let Some(account) = self.get_account(&sysvar::epoch_rewards::id()) {
-            if account.lamports() > 0 {
-                info!(
-                    "burning {} extra lamports in EpochRewards sysvar account at slot {}",
-                    account.lamports(),
-                    self.slot()
-                );
-                self.log_epoch_rewards_sysvar("burn");
-                self.burn_and_purge_account(&sysvar::epoch_rewards::id(), account);
-            }
-        }
+    /// Update EpochRewards sysvar with distributed rewards
+    pub(in crate::bank::partitioned_epoch_rewards) fn set_epoch_rewards_sysvar_to_inactive(&self) {
+        let mut epoch_rewards: sysvar::epoch_rewards::EpochRewards = from_account(
+            &self
+                .get_account(&sysvar::epoch_rewards::id())
+                .unwrap_or_default(),
+        )
+        .unwrap_or_default();
+        assert_eq!(
+            epoch_rewards.distributed_rewards,
+            epoch_rewards.total_rewards
+        );
+        epoch_rewards.active = false;
+
+        self.update_sysvar_account(&sysvar::epoch_rewards::id(), |account| {
+            create_account(
+                &epoch_rewards,
+                self.inherit_specially_retained_account_fields(account),
+            )
+        });
+
+        self.log_epoch_rewards_sysvar("set_inactive");
     }
 }
 
@@ -129,14 +135,17 @@ mod tests {
 
         bank.create_epoch_rewards_sysvar(total_rewards, 10, 42);
         let account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap();
-        assert_eq!(account.lamports(), total_rewards - 10);
+        let expected_balance = bank.get_minimum_balance_for_rent_exemption(account.data().len());
+        // Expected balance is the sysvar rent-exempt balance
+        assert_eq!(account.lamports(), expected_balance);
         let epoch_rewards: sysvar::epoch_rewards::EpochRewards = from_account(&account).unwrap();
         assert_eq!(epoch_rewards, expected_epoch_rewards);
 
         // make a distribution from epoch rewards sysvar
         bank.update_epoch_rewards_sysvar(10);
         let account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap();
-        assert_eq!(account.lamports(), total_rewards - 20);
+        // Balance should not change
+        assert_eq!(account.lamports(), expected_balance);
         let epoch_rewards: sysvar::epoch_rewards::EpochRewards = from_account(&account).unwrap();
         let expected_epoch_rewards = sysvar::epoch_rewards::EpochRewards {
             distribution_starting_block_height: 42,
@@ -148,10 +157,5 @@ mod tests {
             active: true,
         };
         assert_eq!(epoch_rewards, expected_epoch_rewards);
-
-        // burn epoch rewards sysvar
-        bank.burn_and_purge_account(&sysvar::epoch_rewards::id(), account);
-        let account = bank.get_account(&sysvar::epoch_rewards::id());
-        assert!(account.is_none());
     }
 }