Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2.0: SIMD-0118: fix total_rewards for recalculation (backport of #2780) #2794

Merged
merged 2 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions programs/bpf_loader/src/syscalls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +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,
switch_to_new_elf_parser,
last_restart_slot_sysvar, partitioned_epoch_rewards_superfeature, reject_callx_r10,
remaining_compute_units_syscall_enabled, switch_to_new_elf_parser,
},
hash::{Hash, Hasher},
instruction::{AccountMeta, InstructionError, ProcessedSiblingInstruction},
Expand Down Expand Up @@ -274,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());
Expand Down
4 changes: 2 additions & 2 deletions programs/stake/src/stake_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
12 changes: 10 additions & 2 deletions rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,12 +609,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);
Expand Down
39 changes: 22 additions & 17 deletions runtime/src/bank/partitioned_epoch_rewards/calculation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -80,7 +80,7 @@ impl Bank {
distributed_rewards,
distribution_starting_block_height,
num_partitions,
total_points,
point_value,
);

datapoint_info!(
Expand All @@ -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,
Expand All @@ -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) = {
Expand Down Expand Up @@ -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,
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -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,
}
})
}
Expand Down Expand Up @@ -603,7 +600,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,
},
Expand Down Expand Up @@ -990,11 +988,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,
);
Expand All @@ -1014,6 +1015,7 @@ mod tests {
stake_rewards_by_partition: expected_stake_rewards,
..
},
point_value,
..
} = bank.calculate_rewards_for_partitioning(
rewarded_epoch,
Expand All @@ -1037,6 +1039,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);
Expand Down
15 changes: 12 additions & 3 deletions runtime/src/bank/partitioned_epoch_rewards/distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ mod tests {
},
sysvar,
},
solana_stake_program::stake_state,
solana_stake_program::{points::PointValue, stake_state},
solana_vote_program::vote_state,
};

Expand Down Expand Up @@ -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());
Expand Down
59 changes: 44 additions & 15 deletions runtime/src/bank/partitioned_epoch_rewards/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -98,11 +99,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
Expand All @@ -119,12 +133,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
Expand All @@ -136,14 +149,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>,
}
Expand Down Expand Up @@ -183,6 +198,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(
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -454,7 +480,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());
}

Expand Down Expand Up @@ -946,6 +972,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);

Expand Down
Loading