Skip to content

Commit

Permalink
Populate EpochRewards with correct data (solana-labs#763)
Browse files Browse the repository at this point in the history
* Populate EpochRewards::parent_blockhash correctly

* Populate EpochRewards::num_partitions correctly

* Add new internal struct to be extended

* Add CalculateValidatorRewardsResult::total_points

* Add PartitionedRewardsCalculation::total_points

* Add CalculateRewardsAndDistributeVoteRewardsResult::total_points

* Populate EpochRewards::total_points correctly

* Nit: reorder fields to match struct definition

* Use Self::last_blockhash() to get parent_blockhash
  • Loading branch information
CriesofCarrots authored Apr 15, 2024
1 parent aed1a5e commit 180a186
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 33 deletions.
59 changes: 43 additions & 16 deletions runtime/src/bank/partitioned_epoch_rewards/calculation.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use {
super::{
epoch_rewards_hasher::hash_rewards_into_partitions, Bank,
CalculateRewardsAndDistributeVoteRewardsResult, EpochRewardCalculateParamInfo,
PartitionedRewardsCalculation, StakeRewardCalculationPartitioned, VoteRewardsAccounts,
CalculateRewardsAndDistributeVoteRewardsResult, CalculateValidatorRewardsResult,
EpochRewardCalculateParamInfo, PartitionedRewardsCalculation,
StakeRewardCalculationPartitioned, VoteRewardsAccounts,
},
crate::bank::{
PrevEpochInflationRewards, RewardCalcTracer, RewardCalculationEvent, RewardsMetrics,
StakeRewardCalculation, VoteAccount,
VoteAccount,
},
log::info,
rayon::{
Expand Down Expand Up @@ -38,6 +39,7 @@ impl Bank {
let CalculateRewardsAndDistributeVoteRewardsResult {
total_rewards,
distributed_rewards,
total_points,
stake_rewards_by_partition,
} = self.calculate_rewards_and_distribute_vote_rewards(
parent_epoch,
Expand All @@ -49,11 +51,19 @@ impl Bank {
let slot = self.slot();
let credit_start = self.block_height() + self.get_reward_calculation_num_blocks();

let num_partitions = stake_rewards_by_partition.len() as u64;

self.set_epoch_reward_status_active(stake_rewards_by_partition);

// create EpochRewards sysvar that holds the balance of undistributed rewards with
// (total_rewards, distributed_rewards, credit_start), total capital will increase by (total_rewards - distributed_rewards)
self.create_epoch_rewards_sysvar(total_rewards, distributed_rewards, credit_start);
self.create_epoch_rewards_sysvar(
total_rewards,
distributed_rewards,
credit_start,
num_partitions,
total_points,
);

datapoint_info!(
"epoch-rewards-status-update",
Expand Down Expand Up @@ -82,6 +92,7 @@ impl Bank {
foundation_rate,
prev_epoch_duration_in_years,
capitalization,
total_points,
} = self.calculate_rewards_for_partitioning(
prev_epoch,
reward_calc_tracer,
Expand Down Expand Up @@ -150,6 +161,7 @@ impl Bank {
CalculateRewardsAndDistributeVoteRewardsResult {
total_rewards: validator_rewards_paid + total_stake_rewards_lamports,
distributed_rewards: validator_rewards_paid,
total_points,
stake_rewards_by_partition,
}
}
Expand Down Expand Up @@ -198,7 +210,11 @@ impl Bank {

let old_vote_balance_and_staked = self.stakes_cache.stakes().vote_balance_and_staked();

let (vote_account_rewards, mut stake_rewards) = self
let CalculateValidatorRewardsResult {
vote_rewards_accounts: vote_account_rewards,
stake_reward_calculation: mut stake_rewards,
total_points,
} = self
.calculate_validator_rewards(
prev_epoch,
validator_rewards,
Expand Down Expand Up @@ -231,6 +247,7 @@ impl Bank {
foundation_rate,
prev_epoch_duration_in_years,
capitalization,
total_points,
}
}

Expand All @@ -242,7 +259,7 @@ impl Bank {
reward_calc_tracer: Option<impl RewardCalcTracer>,
thread_pool: &ThreadPool,
metrics: &mut RewardsMetrics,
) -> Option<(VoteRewardsAccounts, StakeRewardCalculation)> {
) -> Option<CalculateValidatorRewardsResult> {
let stakes = self.stakes_cache.stakes();
let reward_calculate_param = self.get_epoch_reward_calculate_param_info(&stakes);

Expand All @@ -253,14 +270,21 @@ impl Bank {
metrics,
)
.map(|point_value| {
self.calculate_stake_vote_rewards(
&reward_calculate_param,
rewarded_epoch,
point_value,
thread_pool,
reward_calc_tracer,
metrics,
)
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,
thread_pool,
reward_calc_tracer,
metrics,
);
CalculateValidatorRewardsResult {
vote_rewards_accounts,
stake_reward_calculation,
total_points,
}
})
}

Expand Down Expand Up @@ -478,8 +502,11 @@ mod tests {
&mut rewards_metrics,
);

let vote_rewards = &calculated_rewards.as_ref().unwrap().0;
let stake_rewards = &calculated_rewards.as_ref().unwrap().1;
let vote_rewards = &calculated_rewards.as_ref().unwrap().vote_rewards_accounts;
let stake_rewards = &calculated_rewards
.as_ref()
.unwrap()
.stake_reward_calculation;

let total_vote_rewards: u64 = vote_rewards
.rewards
Expand Down
4 changes: 3 additions & 1 deletion runtime/src/bank/partitioned_epoch_rewards/distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ mod tests {

// Set up epoch_rewards sysvar with rewards with 1e9 lamports to distribute.
let total_rewards = 1_000_000_000;
bank.create_epoch_rewards_sysvar(total_rewards, 0, 42);
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);
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());
Expand Down
14 changes: 13 additions & 1 deletion runtime/src/bank/partitioned_epoch_rewards/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ mod epoch_rewards_hasher;
mod sysvar;

use {
super::Bank,
super::{Bank, StakeRewardCalculation},
crate::{stake_account::StakeAccount, stake_history::StakeHistory},
solana_accounts_db::{
partitioned_rewards::PartitionedEpochRewardsConfig, stake_rewards::StakeReward,
Expand Down Expand Up @@ -50,6 +50,13 @@ pub(super) struct VoteRewardsAccounts {
pub(super) accounts_to_store: Vec<Option<AccountSharedData>>,
}

#[derive(Debug, Default)]
struct CalculateValidatorRewardsResult {
vote_rewards_accounts: VoteRewardsAccounts,
stake_reward_calculation: StakeRewardCalculation,
total_points: u128,
}

/// hold reward calc info to avoid recalculation across functions
pub(super) struct EpochRewardCalculateParamInfo<'a> {
pub(super) stake_history: StakeHistory,
Expand All @@ -69,6 +76,7 @@ pub(super) struct PartitionedRewardsCalculation {
pub(super) foundation_rate: f64,
pub(super) prev_epoch_duration_in_years: f64,
pub(super) capitalization: u64,
total_points: u128,
}

/// result of calculating the stake rewards at beginning of new epoch
Expand All @@ -84,6 +92,10 @@ pub(super) struct CalculateRewardsAndDistributeVoteRewardsResult {
pub(super) total_rewards: u64,
/// distributed vote rewards
pub(super) distributed_rewards: u64,
/// total rewards points calculated for the current epoch, where points
/// equals the sum of (delegated stake * credits observed) for all
/// delegations
pub(super) total_points: u128,
/// stake rewards that still need to be distributed, grouped by partition
pub(super) stake_rewards_by_partition: Vec<StakeRewards>,
}
Expand Down
51 changes: 40 additions & 11 deletions runtime/src/bank/partitioned_epoch_rewards/sysvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,30 @@ impl Bank {
}

/// Create EpochRewards sysvar with calculated rewards
/// This method must be called before a new Bank advances its
/// last_blockhash.
pub(in crate::bank) fn create_epoch_rewards_sysvar(
&self,
total_rewards: u64,
distributed_rewards: u64,
distribution_starting_block_height: u64,
num_partitions: u64,
total_points: u128,
) {
assert!(self.is_partitioned_rewards_code_enabled());

assert!(total_rewards >= distributed_rewards);

let parent_blockhash = self.last_blockhash();

let epoch_rewards = sysvar::epoch_rewards::EpochRewards {
distribution_starting_block_height,
num_partitions,
parent_blockhash,
total_points,
total_rewards,
distributed_rewards,
distribution_starting_block_height,
active: true,
..sysvar::epoch_rewards::EpochRewards::default()
};

self.update_sysvar_account(&sysvar::epoch_rewards::id(), |account| {
Expand Down Expand Up @@ -109,9 +117,10 @@ mod tests {
super::*,
crate::bank::tests::create_genesis_config,
solana_sdk::{
account::ReadableAccount, epoch_schedule::EpochSchedule, feature_set, hash::Hash,
native_token::LAMPORTS_PER_SOL,
account::ReadableAccount, epoch_schedule::EpochSchedule, feature_set,
native_token::LAMPORTS_PER_SOL, pubkey::Pubkey,
},
std::sync::Arc,
};

/// Test `EpochRewards` sysvar creation, distribution, and burning.
Expand All @@ -126,13 +135,15 @@ mod tests {
bank.activate_feature(&feature_set::enable_partitioned_epoch_reward::id());

let total_rewards = 1_000_000_000; // a large rewards so that the sysvar account is rent-exempted.
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

// create epoch rewards sysvar
let expected_epoch_rewards = sysvar::epoch_rewards::EpochRewards {
distribution_starting_block_height: 42,
num_partitions: 0,
parent_blockhash: Hash::default(),
total_points: 0,
num_partitions,
parent_blockhash: bank.last_blockhash(),
total_points,
total_rewards,
distributed_rewards: 10,
active: true,
Expand All @@ -144,14 +155,32 @@ mod tests {
sysvar::epoch_rewards::EpochRewards::default()
);

bank.create_epoch_rewards_sysvar(total_rewards, 10, 42);
bank.create_epoch_rewards_sysvar(total_rewards, 10, 42, num_partitions, total_points);
let account = bank.get_account(&sysvar::epoch_rewards::id()).unwrap();
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);

// Create a child bank to test parent_blockhash
let parent_blockhash = bank.last_blockhash();
let parent_slot = bank.slot();
let bank = Bank::new_from_parent(Arc::new(bank), &Pubkey::default(), parent_slot + 1);
// Also note that running `create_epoch_rewards_sysvar()` against a bank
// with an existing EpochRewards sysvar clobbers the previous values
bank.create_epoch_rewards_sysvar(total_rewards, 10, 42, num_partitions, total_points);

let expected_epoch_rewards = sysvar::epoch_rewards::EpochRewards {
distribution_starting_block_height: 42,
num_partitions,
parent_blockhash,
total_points,
total_rewards,
distributed_rewards: 10,
active: true,
};

let epoch_rewards = bank.get_epoch_rewards_sysvar();
assert_eq!(epoch_rewards, expected_epoch_rewards);

Expand All @@ -163,9 +192,9 @@ mod tests {
let epoch_rewards: sysvar::epoch_rewards::EpochRewards = from_account(&account).unwrap();
let expected_epoch_rewards = sysvar::epoch_rewards::EpochRewards {
distribution_starting_block_height: 42,
num_partitions: 0,
parent_blockhash: Hash::default(),
total_points: 0,
num_partitions,
parent_blockhash,
total_points,
total_rewards,
distributed_rewards: 20,
active: true,
Expand Down
12 changes: 8 additions & 4 deletions runtime/src/bank/sysvar_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ mod tests {
use {
super::*,
solana_sdk::{
feature_set, genesis_config::create_genesis_config, hash::Hash, pubkey::Pubkey,
feature_set, genesis_config::create_genesis_config, pubkey::Pubkey,
sysvar::epoch_rewards::EpochRewards,
},
std::sync::Arc,
Expand Down Expand Up @@ -120,11 +120,13 @@ mod tests {

// inject a reward sysvar for test
bank1.activate_feature(&feature_set::enable_partitioned_epoch_reward::id());
let num_partitions = 2; // num_partitions is arbitrary and unimportant for this test
let total_points = 42_000; // total_points is arbitrary for the purposes of this test
let expected_epoch_rewards = EpochRewards {
distribution_starting_block_height: 42,
num_partitions: 0,
parent_blockhash: Hash::default(),
total_points: 0,
num_partitions,
parent_blockhash: bank1.parent().unwrap().last_blockhash(),
total_points,
total_rewards: 100,
distributed_rewards: 10,
active: true,
Expand All @@ -133,6 +135,8 @@ mod tests {
expected_epoch_rewards.total_rewards,
expected_epoch_rewards.distributed_rewards,
expected_epoch_rewards.distribution_starting_block_height,
num_partitions,
total_points,
);

bank1
Expand Down

0 comments on commit 180a186

Please sign in to comment.