Convert tuples to structs in Stake rewards-calculation functions (#24681)

* Replace opaque tuples with structs in Stake rewards calculation

* Fixup struct and field names, and remove one cumbersome destructuring
This commit is contained in:
Tyera Eulberg 2022-04-27 03:44:10 -04:00 committed by GitHub
parent 2be4ee3619
commit db32549c00
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 131 additions and 47 deletions

View File

@ -182,16 +182,19 @@ fn redeem_stake_rewards(
inflation_point_calc_tracer.as_ref(),
credits_auto_rewind,
)
.map(|(stakers_reward, voters_reward, credits_observed)| {
.map(|calculated_stake_rewards| {
if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer {
inflation_point_calc_tracer(&InflationPointCalculationEvent::CreditsObserved(
stake.credits_observed,
Some(credits_observed),
Some(calculated_stake_rewards.new_credits_observed),
));
}
stake.credits_observed = credits_observed;
stake.delegation.stake += stakers_reward;
(stakers_reward, voters_reward)
stake.credits_observed = calculated_stake_rewards.new_credits_observed;
stake.delegation.stake += calculated_stake_rewards.staker_rewards;
(
calculated_stake_rewards.staker_rewards,
calculated_stake_rewards.voter_rewards,
)
})
}
@ -206,10 +209,17 @@ fn calculate_stake_points(
vote_state,
stake_history,
inflation_point_calc_tracer,
true, // this is safe because this flag shouldn't affect the first
// element (i.e. `points`) of the returned tuple in any way
true, // this is safe because this flag shouldn't affect the
// `points` field of the returned struct in any way
)
.0
.points
}
#[derive(Debug, PartialEq)]
struct CalculatedStakePoints {
points: u128,
new_credits_observed: u64,
force_credits_update_with_skipped_reward: bool,
}
/// for a given stake and vote_state, calculate how many
@ -221,7 +231,7 @@ fn calculate_stake_points_and_credits(
stake_history: Option<&StakeHistory>,
inflation_point_calc_tracer: Option<impl Fn(&InflationPointCalculationEvent)>,
credits_auto_rewind: bool,
) -> (u128, u64, 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
@ -248,7 +258,11 @@ fn calculate_stake_points_and_credits(
// delinquent validator with no differenciation.
// hint with true to indicate some exceptional credits handling is needed
return (0, credits_in_vote, true);
return CalculatedStakePoints {
points: 0,
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
@ -259,7 +273,11 @@ fn calculate_stake_points_and_credits(
}
// don't hint the caller and return current value if credits_auto_rewind is off or
// credits remain to be unchanged (= delinquent)
return (0, credits_in_stake, false);
return CalculatedStakePoints {
points: 0,
new_credits_observed: credits_in_stake,
force_credits_update_with_skipped_reward: false,
};
};
}
@ -303,7 +321,18 @@ fn calculate_stake_points_and_credits(
}
}
(points, new_credits_observed, false)
CalculatedStakePoints {
points,
new_credits_observed,
force_credits_update_with_skipped_reward: false,
}
}
#[derive(Debug, PartialEq)]
struct CalculatedStakeRewards {
staker_rewards: u64,
voter_rewards: u64,
new_credits_observed: u64,
}
/// for a given stake and vote_state, calculate what distributions and what updates should be made
@ -320,16 +349,19 @@ fn calculate_stake_rewards(
stake_history: Option<&StakeHistory>,
inflation_point_calc_tracer: Option<impl Fn(&InflationPointCalculationEvent)>,
credits_auto_rewind: bool,
) -> Option<(u64, u64, u64)> {
) -> Option<CalculatedStakeRewards> {
// ensure to run to trigger (optional) inflation_point_calc_tracer
let (points, credits_observed, mut forced_credits_update_with_skipped_reward) =
calculate_stake_points_and_credits(
stake,
vote_state,
stake_history,
inflation_point_calc_tracer.as_ref(),
credits_auto_rewind,
);
let CalculatedStakePoints {
points,
new_credits_observed,
mut force_credits_update_with_skipped_reward,
} = calculate_stake_points_and_credits(
stake,
vote_state,
stake_history,
inflation_point_calc_tracer.as_ref(),
credits_auto_rewind,
);
// Drive credits_observed forward unconditionally when rewards are disabled
// or when this is the stake's activation epoch
@ -337,17 +369,21 @@ fn calculate_stake_rewards(
if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() {
inflation_point_calc_tracer(&SkippedReason::DisabledInflation.into());
}
forced_credits_update_with_skipped_reward = true;
force_credits_update_with_skipped_reward = true;
} else if stake.delegation.activation_epoch == rewarded_epoch {
// not assert!()-ed; but points should be zero
if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() {
inflation_point_calc_tracer(&SkippedReason::JustActivated.into());
}
forced_credits_update_with_skipped_reward = true;
force_credits_update_with_skipped_reward = true;
}
if forced_credits_update_with_skipped_reward {
return Some((0, 0, credits_observed));
if force_credits_update_with_skipped_reward {
return Some(CalculatedStakeRewards {
staker_rewards: 0,
voter_rewards: 0,
new_credits_observed,
});
}
if points == 0 {
@ -398,7 +434,11 @@ fn calculate_stake_rewards(
return None;
}
Some((staker_rewards, voter_rewards, credits_observed))
Some(CalculatedStakeRewards {
staker_rewards,
voter_rewards,
new_credits_observed,
})
}
pub fn initialize(
@ -2410,7 +2450,11 @@ mod tests {
// this one should be able to collect exactly 2
assert_eq!(
Some((stake.delegation.stake * 2, 0, 2)),
Some(CalculatedStakeRewards {
staker_rewards: stake.delegation.stake * 2,
voter_rewards: 0,
new_credits_observed: 2,
}),
calculate_stake_rewards(
0,
&stake,
@ -2428,7 +2472,11 @@ 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, 2)),
Some(CalculatedStakeRewards {
staker_rewards: stake.delegation.stake,
voter_rewards: 0,
new_credits_observed: 2,
}),
calculate_stake_rewards(
0,
&stake,
@ -2449,7 +2497,11 @@ mod tests {
stake.credits_observed = 2;
// this one should be able to collect the one just added
assert_eq!(
Some((stake.delegation.stake, 0, 3)),
Some(CalculatedStakeRewards {
staker_rewards: stake.delegation.stake,
voter_rewards: 0,
new_credits_observed: 3,
}),
calculate_stake_rewards(
1,
&stake,
@ -2468,7 +2520,11 @@ mod tests {
vote_state.increment_credits(2);
// this one should be able to collect 2 now
assert_eq!(
Some((stake.delegation.stake * 2, 0, 4)),
Some(CalculatedStakeRewards {
staker_rewards: stake.delegation.stake * 2,
voter_rewards: 0,
new_credits_observed: 4,
}),
calculate_stake_rewards(
2,
&stake,
@ -2487,13 +2543,13 @@ mod tests {
// this one should be able to collect everything from t=0 a warmed up stake of 2
// (2 credits at stake of 1) + (1 credit at a stake of 2)
assert_eq!(
Some((
stake.delegation.stake * 2 // epoch 0
Some(CalculatedStakeRewards {
staker_rewards: stake.delegation.stake * 2 // epoch 0
+ stake.delegation.stake // epoch 1
+ stake.delegation.stake, // epoch 2
0,
4
)),
voter_rewards: 0,
new_credits_observed: 4,
}),
calculate_stake_rewards(
2,
&stake,
@ -2547,7 +2603,11 @@ mod tests {
// to advance the stake state's credits_observed field to prevent back-
// paying rewards when inflation is turned on.
assert_eq!(
Some((0, 0, 4)),
Some(CalculatedStakeRewards {
staker_rewards: 0,
voter_rewards: 0,
new_credits_observed: 4,
}),
calculate_stake_rewards(
2,
&stake,
@ -2566,7 +2626,11 @@ mod tests {
// not advancing and inflation is disabled
stake.credits_observed = 4;
assert_eq!(
Some((0, 0, 4)),
Some(CalculatedStakeRewards {
staker_rewards: 0,
voter_rewards: 0,
new_credits_observed: 4,
}),
calculate_stake_rewards(
2,
&stake,
@ -2582,7 +2646,11 @@ mod tests {
);
assert_eq!(
(0, 4, false),
CalculatedStakePoints {
points: 0,
new_credits_observed: 4,
force_credits_update_with_skipped_reward: false,
},
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), true)
);
@ -2591,18 +2659,30 @@ mod tests {
stake.credits_observed = 1000;
// this is old behavior; return the pre-recreation (large) credits from stake account
assert_eq!(
(0, 1000, false),
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!(
(0, 4, true),
CalculatedStakePoints {
points: 0,
new_credits_observed: 4,
force_credits_update_with_skipped_reward: true,
},
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), true)
);
// this is new behavior 2; don't hint when credits both from stake and vote are identical
stake.credits_observed = 4;
assert_eq!(
(0, 4, false),
CalculatedStakePoints {
points: 0,
new_credits_observed: 4,
force_credits_update_with_skipped_reward: false,
},
calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), true)
);
@ -2611,11 +2691,11 @@ mod tests {
stake.credits_observed = 3;
stake.delegation.activation_epoch = 1;
assert_eq!(
Some((
stake.delegation.stake, // epoch 2
0,
4
)),
Some(CalculatedStakeRewards {
staker_rewards: stake.delegation.stake, // epoch 2
voter_rewards: 0,
new_credits_observed: 4,
}),
calculate_stake_rewards(
2,
&stake,
@ -2635,7 +2715,11 @@ mod tests {
stake.delegation.activation_epoch = 2;
stake.credits_observed = 3;
assert_eq!(
Some((0, 0, 4)),
Some(CalculatedStakeRewards {
staker_rewards: 0,
voter_rewards: 0,
new_credits_observed: 4,
}),
calculate_stake_rewards(
2,
&stake,