diff --git a/program-test/tests/sysvar.rs b/program-test/tests/sysvar.rs index 95db7f1a17113d..0bb3292def88da 100644 --- a/program-test/tests/sysvar.rs +++ b/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(()) diff --git a/runtime/src/bank/partitioned_epoch_rewards/distribution.rs b/runtime/src/bank/partitioned_epoch_rewards/distribution.rs index 6975119a8bb409..22297a8abf431b 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/distribution.rs +++ b/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 diff --git a/runtime/src/bank/partitioned_epoch_rewards/mod.rs b/runtime/src/bank/partitioned_epoch_rewards/mod.rs index 3af20f0ddc190c..312de4e987e611 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/mod.rs +++ b/runtime/src/bank/partitioned_epoch_rewards/mod.rs @@ -379,15 +379,19 @@ mod tests { .map(|_| ValidatorVoteKeypairs::new_rand()) .collect::>(); - 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,7 +429,7 @@ 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!( @@ -433,24 +437,49 @@ mod tests { 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); } diff --git a/runtime/src/bank/partitioned_epoch_rewards/sysvar.rs b/runtime/src/bank/partitioned_epoch_rewards/sysvar.rs index 23eb5c986c3512..36e69a51468732 100644 --- a/runtime/src/bank/partitioned_epoch_rewards/sysvar.rs +++ b/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()); } }