Implements logic to pay alpenglow rewards during partitioned epoch rewards

[akhi3030]

Problem

Now that we are tracking AG vote rewards in the vote state, we actually need to pay them out during partitioned epoch rewards (PER).

Summary of Changes

This PR updates PER to actually pay the AG rewards as it observes in the vote state's epoch credits.

This supersedes #11780 as this also handles the migration epoch.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (2)

runtime/src/inflation_rewards/points.rs:401

• The new migration-path logic (CalcEpochType::Migrating + AG_MIGRATION_EPOCH_CREDIT splitting in migrating_epoch_credits_iter) is non-trivial but there doesn’t appear to be a unit test covering the marker-present case (tower credits before marker + AG credits after marker) or the marker-absent case. Adding targeted tests here would help prevent regressions in migration-epoch reward payouts and credit rewinding behavior.

fn migrating_epoch_credits_iter(
    stake: &Stake,
    epoch_credits_iter: impl Iterator<Item = (Epoch, u64, u64)>,
    stake_history: &StakeHistory,
    inflation_point_calc_tracer: Option<impl Fn(&InflationPointCalculationEvent)>,
    new_rate_activation_epoch: Option<Epoch>,
    epoch_stakes: &HashMap<Epoch, VersionedEpochStakes>,
) -> Result<(u128, u128, u64), CalculatedStakePoints> {
    let epoch_credits = epoch_credits_iter.collect::<Vec<_>>();

    let (ag_points, ag_new_credits_observed, ind) = match epoch_credits
        .iter()
        .position(|entry| entry == &AG_MIGRATION_EPOCH_CREDIT)
    {
        None => {
            // No marker should mean that no rewards were paid during alpenglow slots
            (0, 0, epoch_credits.len())
        }
        Some(ind) => {
            let (ag_points, ag_new_credits_observed) = ag_epoch_credits_iter(
                stake,
                epoch_credits[ind + 1..].iter().cloned(),
                stake_history,
                inflation_point_calc_tracer.as_ref(),
                new_rate_activation_epoch,
                epoch_stakes,
            )?;
            (ag_points, ag_new_credits_observed, ind)
        }
    };

    let (tower_points, tower_new_credits_observed) = tower_epoch_credits_iter(
        stake,
        epoch_credits[..ind].iter().cloned(),
        stake_history,
        inflation_point_calc_tracer,
        new_rate_activation_epoch,
    );

    let new_credits_observed = tower_new_credits_observed.max(ag_new_credits_observed);
    Ok((tower_points, ag_points, new_credits_observed))
}

programs/vote/src/vote_state/handler.rs:284

• Making epoch_credits_mut() fully pub exposes the internal epoch_credits: Vec<(Epoch,u64,u64)> representation to all downstream crates, allowing arbitrary mutation that can violate invariants (ordering, MAX_EPOCH_CREDITS_HISTORY trimming, etc.). If this is only needed for runtime vote-reward bookkeeping, consider keeping the vector private (pub(crate)/feature-gated like epoch_credits()) and adding a narrower public API (e.g., a dedicated method to append/increment AG reward credits + marker while maintaining invariants).

    #[cfg_attr(feature = "dev-context-only-utils", qualifiers(pub))]
    pub(crate) fn epoch_credits(&self) -> &Vec<(Epoch, u64, u64)> {
        match &self.target_state {
            TargetVoteState::V4(v4) => &v4.epoch_credits,
        }
    }

    pub fn epoch_credits_mut(&mut self) -> &mut Vec<(Epoch, u64, u64)> {
        match &mut self.target_state {
            TargetVoteState::V4(v4) => &mut v4.epoch_credits,
        }
    }

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

programs/vote/src/vote_state/handler.rs:284

• Making epoch_credits_mut() fully public exposes a raw mutable reference to epoch_credits, allowing external callers to violate invariants (ordering, history truncation, marker entries, etc.). Since existing APIs already provide controlled updates (e.g., increment_credits) and this PR needs special marker-aware behavior, consider keeping this restricted (e.g., pub(crate)/feature-gated like epoch_credits()) and instead adding a dedicated method for the specific update pattern needed by runtime.

    pub fn epoch_credits_mut(&mut self) -> &mut Vec<(Epoch, u64, u64)> {
        match &mut self.target_state {
            TargetVoteState::V4(v4) => &mut v4.epoch_credits,
        }
    }

[AshwinSekar]

minor nit: in the migration epoch if the total_tower_poitns = 0 then we skip paying out the alpenglow reward as well

[AshwinSekar]

Just checking - is it possible that we could have paid the tower credits of the migration epoch but not yet the alpenglow credits for the migration epoch?

Wondering if this filter here would fail in that case and maybe we should allow for

 include entry if:
    entry.0 > migration_epoch
    OR entry is after the migration marker and entry.0 == migration_epoch

If this is not a valid scenario you can ignore

[akhi3030]

Just checking - is it possible that we could have paid the tower credits of the migration epoch but not yet the alpenglow credits for the migration epoch?

Hmm... are you thinking that if this function is called with CalcEpochType::Tower{None} and the epoch credits includes the credits for the migration epoch; then we will pay out the tower rewards for the migration epoch? If that were to happen, then the current code would be quite problematic as it will interpret AG credits and the marker as well as tower.

One idea I have is the following. We make the marker epoch: (1<<63) | migration_epoch. So in any of the three branches here, if we find the marker, we will also know what the migration epoch is and we can ensure that we only handle those when the function is called with CalcEpochType::Migrating.

[akhi3030]

minor nit: in the migration epoch if the total_tower_poitns = 0 then we skip paying out the alpenglow reward as well

Doesn't c43dd83 addresses this?

In the migration epoch, we skip paying if both tower and ag points are zero.

[AshwinSekar]

minor nit: in the migration epoch if the total_tower_poitns = 0 then we skip paying out the alpenglow reward as well

Doesn't c43dd83 addresses this?

In the migration epoch, we skip paying if both tower and ag points are zero.

my comment was for the global one

calculate_stake_rewards(), migration correctly skips only when both tower_points == 0 && ag_points == 0.

but before we get there, calculate_reward_points_partitioned() builds the epoch PointValue by summing calculate_points(), and for CalcEpochType::Migrating, calculate_points() returns res.tower_points.

so if we have migration epoch with 0 tower points and > 0 ag points:

calculate_reward_points_partitioned() returns None here:

  (points > 0).then_some(PointValue { rewards, points })

and calculate_stake_rewards() is never called.

This is a highly unlikely (perhaps impossible) situation though as we needed some votes for the migration to succeed, so i'm fine just ignoring it.

I think i'm mostly fine with this change, once we've shown that it works in the test we should be good to go.

[akhi3030]

In e0ad6e5, I have developed a fairly extensive unit test that exercises the rewards during the migration epoch and asserts that the rewards are as expected. It is testing a bunch of different scenarios with multiple validators each with multiple stakers; different commissions; when AG rewards are available; when they are not, etc.

[akhi3030]

minor nit: in the migration epoch if the total_tower_poitns = 0 then we skip paying out the alpenglow reward as well

Doesn't c43dd83 addresses this?
In the migration epoch, we skip paying if both tower and ag points are zero.

my comment was for the global one

calculate_stake_rewards(), migration correctly skips only when both tower_points == 0 && ag_points == 0.

but before we get there, calculate_reward_points_partitioned() builds the epoch PointValue by summing calculate_points(), and for CalcEpochType::Migrating, calculate_points() returns res.tower_points.

so if we have migration epoch with 0 tower points and > 0 ag points:

calculate_reward_points_partitioned() returns None here:

  (points > 0).then_some(PointValue { rewards, points })

and calculate_stake_rewards() is never called.

This is a highly unlikely (perhaps impossible) situation though as we needed some votes for the migration to succeed, so i'm fine just ignoring it.

I think i'm mostly fine with this change, once we've shown that it works in the test we should be good to go.

Good catch. Yes, I agree that this is a pretty unlikely case and we can ignore it. In fact, my unit test also showed that if tower rewards are not paid, then we do not pay AG rewards.