This repository was archived by the owner on Jan 22, 2025. It is now read-only.
Forcibly reduce credits_observed to the last epoch #13233
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,8 @@ pub enum StakeState { | |
|
|
||
| #[derive(Debug)] | ||
| pub enum InflationPointCalculationEvent { | ||
| CalculatedPoints(u128, u128), | ||
| Stake(u64), | ||
| SplitRewards(u64, u64, u64, PointValue), | ||
| RentExemptReserve(u64), | ||
| Delegation(Delegation), | ||
|
|
@@ -467,14 +468,14 @@ impl Stake { | |
| &mut self, | ||
| point_value: &PointValue, | ||
| vote_state: &VoteState, | ||
| stake_context: Option<(Epoch, &StakeHistory)>, | ||
| inflation_point_calc_tracer: &mut Option<impl FnMut(&InflationPointCalculationEvent)>, | ||
| fix_stake_deactivate: bool, | ||
| ) -> Option<(u64, u64)> { | ||
| self.calculate_rewards( | ||
| point_value, | ||
| vote_state, | ||
| stake_context, | ||
| inflation_point_calc_tracer, | ||
| fix_stake_deactivate, | ||
| ) | ||
|
|
@@ -488,13 +489,13 @@ impl Stake { | |
| pub fn calculate_points( | ||
| &self, | ||
| vote_state: &VoteState, | ||
| stake_context: Option<(Epoch, &StakeHistory)>, | ||
| inflation_point_calc_tracer: &mut Option<impl FnMut(&InflationPointCalculationEvent)>, | ||
| fix_stake_deactivate: bool, | ||
| ) -> u128 { | ||
| self.calculate_points_and_credits( | ||
| vote_state, | ||
| stake_context, | ||
| inflation_point_calc_tracer, | ||
| fix_stake_deactivate, | ||
| ) | ||
|
|
@@ -507,26 +508,42 @@ impl Stake { | |
| pub fn calculate_points_and_credits( | ||
| &self, | ||
| new_vote_state: &VoteState, | ||
| stake_context: Option<(Epoch, &StakeHistory)>, | ||
| inflation_point_calc_tracer: &mut Option<impl FnMut(&InflationPointCalculationEvent)>, | ||
| fix_stake_deactivate: bool, | ||
| ) -> (u128, u64) { | ||
| // if there is no newer credits since observed, return no point | ||
| let mut stake_at_target_epoch = None; | ||
| if let Some((target_epoch, stake_history)) = stake_context { | ||
| let stake = | ||
| self.delegation | ||
| .stake(target_epoch, Some(stake_history), fix_stake_deactivate); | ||
| stake_at_target_epoch = Some(stake); | ||
|
|
||
| if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer { | ||
| inflation_point_calc_tracer(&InflationPointCalculationEvent::Stake(stake)); | ||
| } | ||
| } | ||
|
|
||
| if new_vote_state.credits() <= self.credits_observed { | ||
| return (0, 0); | ||
| } | ||
|
|
||
| let mut points = 0; | ||
| let mut new_credits_observed = self.credits_observed; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Looking into this further... I believe this change can deny small stakes delegated to validators with small, but non-zero commissions ever getting paid out, as per solana/programs/stake/src/stake_state.rs Lines 611 to 616 in d9a07e7
Contributor
Author
There was a problem hiding this comment. yeah! correct! I intentionally done this way now. My two cent is posted here: #13680 (comment) |
||
| if let Some((epoch, final_epoch_credits, initial_epoch_credits)) = | ||
| new_vote_state.epoch_credits().last().copied() | ||
|
ryoqun marked this conversation as resolved.
|
||
| { | ||
| let stake = if let Some((target_epoch, _stake_history)) = stake_context { | ||
| if epoch != target_epoch { | ||
| return (0, 0); | ||
| } | ||
|
Comment on lines
+539
to
+541
Contributor
Author
There was a problem hiding this comment. write a test for this; this bug is later found after manual testing.
Contributor
Author
There was a problem hiding this comment. well, if we don't exclude historical vote_state's epoch credits by doing like this early |
||
| stake_at_target_epoch.unwrap() | ||
| } else { | ||
| self.delegation.stake(epoch, None, fix_stake_deactivate) | ||
| }; | ||
| let stake = u128::from(stake); | ||
|
|
||
| // figure out how much this stake has seen that | ||
| // for which the vote account has a record | ||
|
|
@@ -535,24 +552,23 @@ impl Stake { | |
| final_epoch_credits - initial_epoch_credits | ||
| } else if self.credits_observed < final_epoch_credits { | ||
| // the staker registered sometime during the epoch, partial credit | ||
| final_epoch_credits - self.credits_observed | ||
| } else { | ||
| // the staker has already observed or been redeemed this epoch | ||
| // or was activated after this epoch | ||
| 0 | ||
| }; | ||
| let earned_credits = u128::from(earned_credits); | ||
|
|
||
| // try to update credits_observed | ||
| new_credits_observed = final_epoch_credits; | ||
|
|
||
| // finally calculate points for this epoch | ||
| points += stake * earned_credits; | ||
|
|
||
| if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer { | ||
| inflation_point_calc_tracer(&InflationPointCalculationEvent::CalculatedPoints( | ||
| points, | ||
| earned_credits, | ||
| )); | ||
| } | ||
|
|
@@ -571,13 +587,13 @@ impl Stake { | |
| &self, | ||
| point_value: &PointValue, | ||
| vote_state: &VoteState, | ||
| stake_context: Option<(Epoch, &StakeHistory)>, | ||
| inflation_point_calc_tracer: &mut Option<impl FnMut(&InflationPointCalculationEvent)>, | ||
| fix_stake_deactivate: bool, | ||
| ) -> Option<(u64, u64, u64)> { | ||
| let (points, credits_observed) = self.calculate_points_and_credits( | ||
| vote_state, | ||
| stake_context, | ||
| inflation_point_calc_tracer, | ||
| fix_stake_deactivate, | ||
| ); | ||
|
|
@@ -1100,7 +1116,7 @@ pub fn redeem_rewards( | |
| stake_account: &mut Account, | ||
| vote_account: &mut Account, | ||
| point_value: &PointValue, | ||
| stake_context: Option<(Epoch, &StakeHistory)>, | ||
| inflation_point_calc_tracer: &mut Option<impl FnMut(&InflationPointCalculationEvent)>, | ||
| fix_stake_deactivate: bool, | ||
| ) -> Result<(u64, u64), InstructionError> { | ||
|
|
@@ -1119,7 +1135,7 @@ pub fn redeem_rewards( | |
| if let Some((stakers_reward, voters_reward)) = stake.redeem_rewards( | ||
| point_value, | ||
| &vote_state, | ||
| stake_context, | ||
| inflation_point_calc_tracer, | ||
| fix_stake_deactivate, | ||
| ) { | ||
|
|
@@ -1141,7 +1157,7 @@ pub fn redeem_rewards( | |
| pub fn calculate_points( | ||
| stake_account: &Account, | ||
| vote_account: &Account, | ||
| stake_context: Option<(Epoch, &StakeHistory)>, | ||
| fix_stake_deactivate: bool, | ||
| ) -> Result<u128, InstructionError> { | ||
| if let StakeState::Stake(_meta, stake) = stake_account.state()? { | ||
|
|
@@ -1150,7 +1166,7 @@ pub fn calculate_points( | |
|
|
||
| Ok(stake.calculate_points( | ||
| &vote_state, | ||
| stake_context, | ||
| &mut null_tracer(), | ||
| fix_stake_deactivate, | ||
| )) | ||
|
|
@@ -2853,13 +2869,17 @@ mod tests { | |
| ) | ||
| ); | ||
|
|
||
| let mut current_credits = 0; | ||
|
|
||
| // put 2 credits in at epoch 0 | ||
| vote_state.increment_credits(0); | ||
| current_credits += 1; | ||
| vote_state.increment_credits(0); | ||
| current_credits += 1; | ||
|
|
||
| // this one should be able to collect exactly 2 | ||
| assert_eq!( | ||
| Some((stake.delegation.stake * 2, 0, current_credits)), | ||
| stake.calculate_rewards( | ||
| &PointValue { | ||
| rewards: 2, | ||
|
|
@@ -2875,7 +2895,7 @@ mod tests { | |
| stake.credits_observed = 1; | ||
| // this one should be able to collect exactly 1 (already observed one) | ||
| assert_eq!( | ||
| Some((stake.delegation.stake, 0, current_credits)), | ||
| stake.calculate_rewards( | ||
| &PointValue { | ||
| rewards: 1, | ||
|
|
@@ -2890,11 +2910,12 @@ mod tests { | |
|
|
||
| // put 1 credit in epoch 1 | ||
| vote_state.increment_credits(1); | ||
| current_credits += 1; | ||
|
|
||
| stake.credits_observed = 2; | ||
| // this one should be able to collect the one just added | ||
| assert_eq!( | ||
| Some((stake.delegation.stake, 0, current_credits)), | ||
| stake.calculate_rewards( | ||
| &PointValue { | ||
| rewards: 2, | ||
|
|
@@ -2909,9 +2930,12 @@ mod tests { | |
|
|
||
| // put 1 credit in epoch 2 | ||
| vote_state.increment_credits(2); | ||
| current_credits += 1; | ||
| vote_state.increment_credits(2); | ||
| current_credits += 1; | ||
| // this one should be able to collect 2 now | ||
| assert_eq!( | ||
| Some((stake.delegation.stake * 2, 0, current_credits)), | ||
| stake.calculate_rewards( | ||
| &PointValue { | ||
| rewards: 2, | ||
|
|
@@ -2925,15 +2949,13 @@ mod tests { | |
| ); | ||
|
|
||
| stake.credits_observed = 0; | ||
| // even if creidts_observed is so small, | ||
| // this one should collect only the last epoch | ||
| assert_eq!( | ||
| Some(( | ||
| stake.delegation.stake * 2, // only epoch 2 | ||
| 0, | ||
| current_credits | ||
| )), | ||
| stake.calculate_rewards( | ||
| &PointValue { | ||
|
|
@@ -2951,7 +2973,7 @@ mod tests { | |
| // verify that None comes back on small redemptions where no one gets paid | ||
| vote_state.commission = 1; | ||
| assert_eq!( | ||
| None, // would be Some((0, 2 * 1 + 1 * 2, current_credits)), | ||
| stake.calculate_rewards( | ||
| &PointValue { | ||
| rewards: 4, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move comment along...