Clean up credits_auto_rewind feature (#32211)

* Remove credits_auto_rewind feature logic from stake program

* Obey clean-up orders

* Clippy prefers this match

* Remove credits_auto_rewind feature helpers from Bank

* Comment nit for language, and shrink to fit on one line
This commit is contained in:
Tyera 2023-06-24 10:26:27 -06:00 committed by GitHub
parent 0b8af56924
commit 02eaaae513
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 59 deletions

View File

@ -33,7 +33,7 @@ use {
},
},
solana_vote_program::vote_state::{self, VoteState, VoteStateVersions},
std::{collections::HashSet, convert::TryFrom},
std::{cmp::Ordering, collections::HashSet, convert::TryFrom},
};
#[derive(Debug)]
@ -183,7 +183,6 @@ fn redeem_stake_rewards(
vote_state: &VoteState,
stake_history: Option<&StakeHistory>,
inflation_point_calc_tracer: Option<impl Fn(&InflationPointCalculationEvent)>,
credits_auto_rewind: bool,
) -> Option<(u64, u64)> {
if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() {
inflation_point_calc_tracer(&InflationPointCalculationEvent::CreditsObserved(
@ -198,7 +197,6 @@ fn redeem_stake_rewards(
vote_state,
stake_history,
inflation_point_calc_tracer.as_ref(),
credits_auto_rewind,
)
.map(|calculated_stake_rewards| {
if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer {
@ -227,8 +225,6 @@ fn calculate_stake_points(
vote_state,
stake_history,
inflation_point_calc_tracer,
true, // this is safe because this flag shouldn't affect the
// `points` field of the returned struct in any way
)
.points
}
@ -248,13 +244,12 @@ fn calculate_stake_points_and_credits(
new_vote_state: &VoteState,
stake_history: Option<&StakeHistory>,
inflation_point_calc_tracer: Option<impl Fn(&InflationPointCalculationEvent)>,
credits_auto_rewind: bool,
) -> CalculatedStakePoints {
let credits_in_stake = stake.credits_observed;
let credits_in_vote = new_vote_state.credits();
// if there is no newer credits since observed, return no point
if credits_in_vote <= credits_in_stake {
if credits_auto_rewind && credits_in_vote < credits_in_stake {
match credits_in_vote.cmp(&credits_in_stake) {
Ordering::Less => {
if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() {
inflation_point_calc_tracer(&SkippedReason::ZeroCreditsAndReturnRewinded.into());
}
@ -281,22 +276,19 @@ fn calculate_stake_points_and_credits(
new_credits_observed: credits_in_vote,
force_credits_update_with_skipped_reward: true,
};
} else {
// change the above `else` to `else if credits_in_vote == credits_in_stake`
// (and remove the outermost enclosing `if`) when cleaning credits_auto_rewind
// after activation
}
Ordering::Equal => {
if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() {
inflation_point_calc_tracer(&SkippedReason::ZeroCreditsAndReturnCurrent.into());
}
// don't hint the caller and return current value if credits_auto_rewind is off or
// credits remain to be unchanged (= delinquent)
// don't hint caller and return current value if credits remain unchanged (= delinquent)
return CalculatedStakePoints {
points: 0,
new_credits_observed: credits_in_stake,
force_credits_update_with_skipped_reward: false,
};
};
}
Ordering::Greater => {}
}
let mut points = 0;
@ -366,7 +358,6 @@ fn calculate_stake_rewards(
vote_state: &VoteState,
stake_history: Option<&StakeHistory>,
inflation_point_calc_tracer: Option<impl Fn(&InflationPointCalculationEvent)>,
credits_auto_rewind: bool,
) -> Option<CalculatedStakeRewards> {
// ensure to run to trigger (optional) inflation_point_calc_tracer
let CalculatedStakePoints {
@ -378,7 +369,6 @@ fn calculate_stake_rewards(
vote_state,
stake_history,
inflation_point_calc_tracer.as_ref(),
credits_auto_rewind,
);
// Drive credits_observed forward unconditionally when rewards are disabled
@ -1554,7 +1544,6 @@ pub fn redeem_rewards(
point_value: &PointValue,
stake_history: Option<&StakeHistory>,
inflation_point_calc_tracer: Option<impl Fn(&InflationPointCalculationEvent)>,
credits_auto_rewind: bool,
) -> Result<(u64, u64), InstructionError> {
if let StakeState::Stake(meta, mut stake) = stake_state {
if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() {
@ -1578,7 +1567,6 @@ pub fn redeem_rewards(
vote_state,
stake_history,
inflation_point_calc_tracer,
credits_auto_rewind,
) {
stake_account.checked_add_lamports(stakers_reward)?;
stake_account.set_state(&StakeState::Stake(meta, stake))?;
@ -2510,7 +2498,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2531,7 +2518,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2569,7 +2555,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2613,7 +2598,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2638,7 +2622,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2660,7 +2643,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2685,7 +2667,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2708,7 +2689,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2733,7 +2713,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2752,7 +2731,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
vote_state.commission = 99;
@ -2768,7 +2746,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2791,7 +2768,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2814,7 +2790,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2824,21 +2799,12 @@ mod tests {
new_credits_observed: 4,
force_credits_update_with_skipped_reward: false,
},
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), true)
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer())
);
// credits_observed is auto-rewinded when vote_state credits are assumed to have been
// recreated
stake.credits_observed = 1000;
// this is old behavior; return the pre-recreation (large) credits from stake account
assert_eq!(
CalculatedStakePoints {
points: 0,
new_credits_observed: 1000,
force_credits_update_with_skipped_reward: false,
},
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), false)
);
// this is new behavior 1; return the post-recreation rewinded credits from the vote account
assert_eq!(
CalculatedStakePoints {
@ -2846,7 +2812,7 @@ mod tests {
new_credits_observed: 4,
force_credits_update_with_skipped_reward: true,
},
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), true)
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer())
);
// this is new behavior 2; don't hint when credits both from stake and vote are identical
stake.credits_observed = 4;
@ -2856,7 +2822,7 @@ mod tests {
new_credits_observed: 4,
force_credits_update_with_skipped_reward: false,
},
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), true)
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer())
);
// get rewards and credits observed when not the activation epoch
@ -2879,7 +2845,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
@ -2903,7 +2868,6 @@ mod tests {
&vote_state,
None,
null_tracer(),
true,
)
);
}

View File

@ -2833,7 +2833,6 @@ impl Bank {
prev_epoch,
validator_rewards,
reward_calc_tracer,
self.credits_auto_rewind(),
thread_pool,
metrics,
update_rewards_from_cached_accounts,
@ -3270,7 +3269,6 @@ impl Bank {
rewarded_epoch: Epoch,
rewards: u64,
reward_calc_tracer: Option<impl RewardCalcTracer>,
credits_auto_rewind: bool,
thread_pool: &ThreadPool,
metrics: &mut RewardsMetrics,
update_rewards_from_cached_accounts: bool,
@ -3296,7 +3294,6 @@ impl Bank {
vote_with_stake_delegations_map,
rewarded_epoch,
point_value,
credits_auto_rewind,
&stake_history,
thread_pool,
reward_calc_tracer.as_ref(),
@ -3578,7 +3575,6 @@ impl Bank {
reward_calc_tracer: Option<impl RewardCalcTracer>,
metrics: &mut RewardsMetrics,
) -> (VoteRewardsAccounts, StakeRewardCalculation) {
let credits_auto_rewind = self.credits_auto_rewind();
let EpochRewardCalculateParamInfo {
stake_history,
stake_delegations,
@ -3646,7 +3642,6 @@ impl Bank {
&point_value,
Some(stake_history),
reward_calc_tracer.as_ref(),
credits_auto_rewind,
);
let post_lamport = stake_account.lamports();
@ -3713,7 +3708,6 @@ impl Bank {
vote_with_stake_delegations_map: DashMap<Pubkey, VoteWithStakeDelegations>,
rewarded_epoch: Epoch,
point_value: PointValue,
credits_auto_rewind: bool,
stake_history: &StakeHistory,
thread_pool: &ThreadPool,
reward_calc_tracer: Option<impl RewardCalcTracer>,
@ -3765,7 +3759,6 @@ impl Bank {
&point_value,
Some(stake_history),
reward_calc_tracer.as_ref(),
credits_auto_rewind,
);
if let Ok((stakers_reward, voters_reward)) = redeemed {
// track voter rewards
@ -8420,11 +8413,6 @@ impl Bank {
.is_active(&feature_set::prevent_rent_paying_rent_recipients::id())
}
pub fn credits_auto_rewind(&self) -> bool {
self.feature_set
.is_active(&feature_set::credits_auto_rewind::id())
}
pub fn read_cost_tracker(&self) -> LockResult<RwLockReadGuard<CostTracker>> {
self.cost_tracker.read()
}