Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

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

Merged
mergify merged 2 commits into from
Apr 27, 2022
Merged

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da89e5c
Replace opaque tuples with structs in Stake rewards calculation
Apr 26, 2022
ecf9603
Fixup struct and field names, and remove one cumbersome destructuring
Apr 26, 2022
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
178 changes: 131 additions & 47 deletions programs/stake/src/stake_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
})
}

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
};
};
}

Expand Down Expand Up @@ -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
Expand All @@ -320,34 +349,41 @@ 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
if point_value.rewards == 0 {
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 {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)
);

Expand All @@ -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)
);

Expand All @@ -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,
Expand All @@ -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,
Expand Down