stake: Advance `credits_observed` on activation epoch (#19309)

* stake: Advance `credits_observed` on activation epoch

* Add test for merging stakes just after activation

program-test/tests/warp.rs

@@ -2,7 +2,8 @@
 use {
     assert_matches::assert_matches,
     bincode::deserialize,
-    solana_program_test::{processor, ProgramTest, ProgramTestError},
+    solana_banks_client::BanksClient,
+    solana_program_test::{processor, ProgramTest, ProgramTestContext, ProgramTestError},
     solana_sdk::{
         account_info::{next_account_info, AccountInfo},
         clock::Clock,
@@ -34,6 +35,75 @@ use {
 // Use a big number to be sure that we get the right error
 const WRONG_SLOT_ERROR: u32 = 123456;
 
+async fn setup_stake(
+    context: &mut ProgramTestContext,
+    user: &Keypair,
+    vote_address: &Pubkey,
+    stake_lamports: u64,
+) -> Pubkey {
+    let stake_keypair = Keypair::new();
+    let transaction = Transaction::new_signed_with_payer(
+        &stake_instruction::create_account_and_delegate_stake(
+            &context.payer.pubkey(),
+            &stake_keypair.pubkey(),
+            vote_address,
+            &Authorized::auto(&user.pubkey()),
+            &Lockup::default(),
+            stake_lamports,
+        ),
+        Some(&context.payer.pubkey()),
+        &vec![&context.payer, &stake_keypair, user],
+        context.last_blockhash,
+    );
+    context
+        .banks_client
+        .process_transaction(transaction)
+        .await
+        .unwrap();
+    stake_keypair.pubkey()
+}
+
+async fn setup_vote(context: &mut ProgramTestContext) -> Pubkey {
+    // warp once to make sure stake config doesn't get rent-collected
+    context.warp_to_slot(100).unwrap();
+    let mut instructions = vec![];
+    let validator_keypair = Keypair::new();
+    instructions.push(system_instruction::create_account(
+        &context.payer.pubkey(),
+        &validator_keypair.pubkey(),
+        42,
+        0,
+        &system_program::id(),
+    ));
+    let vote_lamports = Rent::default().minimum_balance(VoteState::size_of());
+    let vote_keypair = Keypair::new();
+    let user_keypair = Keypair::new();
+    instructions.append(&mut vote_instruction::create_account(
+        &context.payer.pubkey(),
+        &vote_keypair.pubkey(),
+        &VoteInit {
+            node_pubkey: validator_keypair.pubkey(),
+            authorized_voter: user_keypair.pubkey(),
+            ..VoteInit::default()
+        },
+        vote_lamports,
+    ));
+
+    let transaction = Transaction::new_signed_with_payer(
+        &instructions,
+        Some(&context.payer.pubkey()),
+        &vec![&context.payer, &validator_keypair, &vote_keypair],
+        context.last_blockhash,
+    );
+    context
+        .banks_client
+        .process_transaction(transaction)
+        .await
+        .unwrap();
+
+    vote_keypair.pubkey()
+}
+
 fn process_instruction(
     _program_id: &Pubkey,
     accounts: &[AccountInfo],
@@ -166,63 +236,17 @@ async fn rent_collected_from_warp() {
 async fn stake_rewards_from_warp() {
     // Initialize and start the test network
     let program_test = ProgramTest::default();
-
     let mut context = program_test.start_with_context().await;
-    // warp once to make sure stake config doesn't get rent-collected
+    let vote_address = setup_vote(&mut context).await;
-    context.warp_to_slot(100).unwrap();
-    let mut instructions = vec![];
-    let validator_keypair = Keypair::new();
-    instructions.push(system_instruction::create_account(
-        &context.payer.pubkey(),
-        &validator_keypair.pubkey(),
-        42,
-        0,
-        &system_program::id(),
-    ));
-    let vote_lamports = Rent::default().minimum_balance(VoteState::size_of());
-    let vote_keypair = Keypair::new();
-    let user_keypair = Keypair::new();
-    instructions.append(&mut vote_instruction::create_account(
-        &context.payer.pubkey(),
-        &vote_keypair.pubkey(),
-        &VoteInit {
-            node_pubkey: validator_keypair.pubkey(),
-            authorized_voter: user_keypair.pubkey(),
-            ..VoteInit::default()
-        },
-        vote_lamports,
-    ));
 
-    let stake_keypair = Keypair::new();
+    let user_keypair = Keypair::new();
     let stake_lamports = 1_000_000_000_000;
-    instructions.append(&mut stake_instruction::create_account_and_delegate_stake(
+    let stake_address =
-        &context.payer.pubkey(),
+        setup_stake(&mut context, &user_keypair, &vote_address, stake_lamports).await;
-        &stake_keypair.pubkey(),
+
-        &vote_keypair.pubkey(),
-        &Authorized::auto(&user_keypair.pubkey()),
-        &Lockup::default(),
-        stake_lamports,
-    ));
-    let transaction = Transaction::new_signed_with_payer(
-        &instructions,
-        Some(&context.payer.pubkey()),
-        &vec![
-            &context.payer,
-            &validator_keypair,
-            &vote_keypair,
-            &stake_keypair,
-            &user_keypair,
-        ],
-        context.last_blockhash,
-    );
-    context
-        .banks_client
-        .process_transaction(transaction)
-        .await
-        .unwrap();
     let account = context
         .banks_client
-        .get_account(stake_keypair.pubkey())
+        .get_account(stake_address)
         .await
         .expect("account exists")
         .unwrap();
@@ -233,13 +257,13 @@ async fn stake_rewards_from_warp() {
     context.warp_to_slot(first_normal_slot).unwrap();
     let account = context
         .banks_client
-        .get_account(stake_keypair.pubkey())
+        .get_account(stake_address)
         .await
         .expect("account exists")
         .unwrap();
     assert_eq!(account.lamports, stake_lamports);
 
-    context.increment_vote_account_credits(&vote_keypair.pubkey(), 100);
+    context.increment_vote_account_credits(&vote_address, 100);
 
     // go forward and see that rewards have been distributed
     let slots_per_epoch = context.genesis_config().epoch_schedule.slots_per_epoch;
@@ -249,7 +273,7 @@ async fn stake_rewards_from_warp() {
 
     let account = context
         .banks_client
-        .get_account(stake_keypair.pubkey())
+        .get_account(stake_address)
         .await
         .expect("account exists")
         .unwrap();
@@ -281,3 +305,112 @@ async fn stake_rewards_from_warp() {
         (_, 0, 0)
     );
 }
+
+async fn check_credits_observed(
+    banks_client: &mut BanksClient,
+    stake_address: Pubkey,
+    expected_credits: u64,
+) {
+    let stake_account = banks_client
+        .get_account(stake_address)
+        .await
+        .unwrap()
+        .unwrap();
+    let stake_state: StakeState = deserialize(&stake_account.data).unwrap();
+    assert_eq!(
+        stake_state.stake().unwrap().credits_observed,
+        expected_credits
+    );
+}
+
+#[tokio::test]
+async fn stake_merge_immediately_after_activation() {
+    let program_test = ProgramTest::default();
+    let mut context = program_test.start_with_context().await;
+    let vote_address = setup_vote(&mut context).await;
+    context.increment_vote_account_credits(&vote_address, 100);
+
+    let first_normal_slot = context.genesis_config().epoch_schedule.first_normal_slot;
+    let slots_per_epoch = context.genesis_config().epoch_schedule.slots_per_epoch;
+    let mut current_slot = first_normal_slot + slots_per_epoch;
+    context.warp_to_slot(current_slot).unwrap();
+
+    // this is annoying, but if no stake has earned rewards, the bank won't
+    // iterate through the stakes at all, which means we can only test the
+    // behavior of advancing credits observed if another stake is earning rewards
+
+    // make a base stake which receives rewards
+    let user_keypair = Keypair::new();
+    let stake_lamports = 1_000_000_000_000;
+    let base_stake_address =
+        setup_stake(&mut context, &user_keypair, &vote_address, stake_lamports).await;
+    check_credits_observed(&mut context.banks_client, base_stake_address, 100).await;
+    context.increment_vote_account_credits(&vote_address, 100);
+
+    current_slot += slots_per_epoch;
+    context.warp_to_slot(current_slot).unwrap();
+
+    // make another stake which will just have its credits observed advanced
+    let absorbed_stake_address =
+        setup_stake(&mut context, &user_keypair, &vote_address, stake_lamports).await;
+    // the new stake is at the right value
+    check_credits_observed(&mut context.banks_client, absorbed_stake_address, 200).await;
+    // the base stake hasn't been moved forward because no rewards were earned
+    check_credits_observed(&mut context.banks_client, base_stake_address, 100).await;
+
+    context.increment_vote_account_credits(&vote_address, 100);
+    current_slot += slots_per_epoch;
+    context.warp_to_slot(current_slot).unwrap();
+
+    // check that base stake has earned rewards and credits moved forward
+    let stake_account = context
+        .banks_client
+        .get_account(base_stake_address)
+        .await
+        .unwrap()
+        .unwrap();
+    let stake_state: StakeState = deserialize(&stake_account.data).unwrap();
+    assert_eq!(stake_state.stake().unwrap().credits_observed, 300);
+    assert!(stake_account.lamports > stake_lamports);
+
+    // check that new stake hasn't earned rewards, but that credits_observed have been advanced
+    let stake_account = context
+        .banks_client
+        .get_account(absorbed_stake_address)
+        .await
+        .unwrap()
+        .unwrap();
+    let stake_state: StakeState = deserialize(&stake_account.data).unwrap();
+    assert_eq!(stake_state.stake().unwrap().credits_observed, 300);
+    assert_eq!(stake_account.lamports, stake_lamports);
+
+    // sanity-check that the activation epoch was actually last epoch
+    let clock_account = context
+        .banks_client
+        .get_account(clock::id())
+        .await
+        .unwrap()
+        .unwrap();
+    let clock: Clock = deserialize(&clock_account.data).unwrap();
+    assert_eq!(
+        clock.epoch,
+        stake_state.delegation().unwrap().activation_epoch + 1
+    );
+
+    // sanity-check that it's possible to merge the just-activated stake with the older stake!
+    let transaction = Transaction::new_signed_with_payer(
+        &stake_instruction::merge(
+            &base_stake_address,
+            &absorbed_stake_address,
+            &user_keypair.pubkey(),
+        ),
+        Some(&context.payer.pubkey()),
+        &vec![&context.payer, &user_keypair],
+        context.last_blockhash,
+    );
+    context
+        .banks_client
+        .process_transaction(transaction)
+        .await
+        .unwrap();
+}

programs/stake/src/stake_state.rs

@@ -156,11 +156,13 @@ pub struct PointValue {
 }
 
 fn redeem_stake_rewards(
+    rewarded_epoch: Epoch,
     stake: &mut Stake,
     point_value: &PointValue,
     vote_state: &VoteState,
     stake_history: Option<&StakeHistory>,
     inflation_point_calc_tracer: &mut Option<impl FnMut(&InflationPointCalculationEvent)>,
+    fix_activating_credits_observed: bool,
 ) -> Option<(u64, u64)> {
     if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer {
         inflation_point_calc_tracer(&InflationPointCalculationEvent::CreditsObserved(
@@ -169,11 +171,13 @@ fn redeem_stake_rewards(
         ));
     }
     calculate_stake_rewards(
+        rewarded_epoch,
         stake,
         point_value,
         vote_state,
         stake_history,
         inflation_point_calc_tracer,
+        fix_activating_credits_observed,
     )
     .map(|(stakers_reward, voters_reward, credits_observed)| {
         if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer {
@@ -270,11 +274,13 @@ fn calculate_stake_points_and_credits(
 ///   * new value for credits_observed in the stake
 /// returns None if there's no payout or if any deserved payout is < 1 lamport
 fn calculate_stake_rewards(
+    rewarded_epoch: Epoch,
     stake: &Stake,
     point_value: &PointValue,
     vote_state: &VoteState,
     stake_history: Option<&StakeHistory>,
     inflation_point_calc_tracer: &mut Option<impl FnMut(&InflationPointCalculationEvent)>,
+    fix_activating_credits_observed: bool,
 ) -> Option<(u64, u64, u64)> {
     let (points, credits_observed) = calculate_stake_points_and_credits(
         stake,
@@ -284,7 +290,10 @@ fn calculate_stake_rewards(
     );
 
     // Drive credits_observed forward unconditionally when rewards are disabled
-    if point_value.rewards == 0 {
+    // or when this is the stake's activation epoch
+    if point_value.rewards == 0
+        || (fix_activating_credits_observed && stake.delegation.activation_epoch == rewarded_epoch)
+    {
         return Some((0, 0, credits_observed));
     }
 
@@ -1098,6 +1107,7 @@ pub fn redeem_rewards(
     point_value: &PointValue,
     stake_history: Option<&StakeHistory>,
     inflation_point_calc_tracer: &mut Option<impl FnMut(&InflationPointCalculationEvent)>,
+    fix_activating_credits_observed: bool,
 ) -> Result<(u64, u64), InstructionError> {
     if let StakeState::Stake(meta, mut stake) = stake_account.state()? {
         if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer {
@@ -1115,11 +1125,13 @@ pub fn redeem_rewards(
         }
 
         if let Some((stakers_reward, voters_reward)) = redeem_stake_rewards(
+            rewarded_epoch,
             &mut stake,
             point_value,
             vote_state,
             stake_history,
             inflation_point_calc_tracer,
+            fix_activating_credits_observed,
         ) {
             stake_account.checked_add_lamports(stakers_reward)?;
             vote_account.checked_add_lamports(voters_reward)?;
@@ -3379,6 +3391,7 @@ mod tests {
         assert_eq!(
             None,
             redeem_stake_rewards(
+                0,
                 &mut stake,
                 &PointValue {
                     rewards: 1_000_000_000,
@@ -3387,6 +3400,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3398,6 +3412,7 @@ mod tests {
         assert_eq!(
             Some((stake_lamports * 2, 0)),
             redeem_stake_rewards(
+                0,
                 &mut stake,
                 &PointValue {
                     rewards: 1,
@@ -3406,6 +3421,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3434,6 +3450,7 @@ mod tests {
         assert_eq!(
             None,
             calculate_stake_rewards(
+                0,
                 &stake,
                 &PointValue {
                     rewards: 1_000_000_000,
@@ -3442,6 +3459,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3476,6 +3494,7 @@ mod tests {
         assert_eq!(
             None,
             calculate_stake_rewards(
+                0,
                 &stake,
                 &PointValue {
                     rewards: 1_000_000_000,
@@ -3484,6 +3503,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3495,6 +3515,7 @@ mod tests {
         assert_eq!(
             Some((stake.delegation.stake * 2, 0, 2)),
             calculate_stake_rewards(
+                0,
                 &stake,
                 &PointValue {
                     rewards: 2,
@@ -3503,6 +3524,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3511,6 +3533,7 @@ mod tests {
         assert_eq!(
             Some((stake.delegation.stake, 0, 2)),
             calculate_stake_rewards(
+                0,
                 &stake,
                 &PointValue {
                     rewards: 1,
@@ -3519,6 +3542,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3530,6 +3554,7 @@ mod tests {
         assert_eq!(
             Some((stake.delegation.stake, 0, 3)),
             calculate_stake_rewards(
+                1,
                 &stake,
                 &PointValue {
                     rewards: 2,
@@ -3538,6 +3563,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3547,6 +3573,7 @@ mod tests {
         assert_eq!(
             Some((stake.delegation.stake * 2, 0, 4)),
             calculate_stake_rewards(
+                2,
                 &stake,
                 &PointValue {
                     rewards: 2,
@@ -3555,6 +3582,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3570,6 +3598,7 @@ mod tests {
                 4
             )),
             calculate_stake_rewards(
+                2,
                 &stake,
                 &PointValue {
                     rewards: 4,
@@ -3578,6 +3607,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3587,6 +3617,7 @@ mod tests {
         assert_eq!(
             None, // would be Some((0, 2 * 1 + 1 * 2, 4)),
             calculate_stake_rewards(
+                2,
                 &stake,
                 &PointValue {
                     rewards: 4,
@@ -3595,12 +3626,14 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
         vote_state.commission = 99;
         assert_eq!(
             None, // would be Some((0, 2 * 1 + 1 * 2, 4)),
             calculate_stake_rewards(
+                2,
                 &stake,
                 &PointValue {
                     rewards: 4,
@@ -3609,6 +3642,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3618,6 +3652,7 @@ mod tests {
         assert_eq!(
             Some((0, 0, 4)),
             calculate_stake_rewards(
+                2,
                 &stake,
                 &PointValue {
                     rewards: 0,
@@ -3626,6 +3661,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3635,6 +3671,7 @@ mod tests {
         assert_eq!(
             Some((0, 0, 4)),
             calculate_stake_rewards(
+                2,
                 &stake,
                 &PointValue {
                     rewards: 0,
@@ -3643,6 +3680,7 @@ mod tests {
                 &vote_state,
                 None,
                 &mut null_tracer(),
+                true,
             )
         );
 
@@ -3651,6 +3689,50 @@ mod tests {
             (0, 4),
             calculate_stake_points_and_credits(&stake, &vote_state, None, &mut null_tracer())
         );
+
+        // get rewards and credits observed when not the activation epoch
+        vote_state.commission = 0;
+        stake.credits_observed = 3;
+        stake.delegation.activation_epoch = 1;
+        assert_eq!(
+            Some((
+                stake.delegation.stake, // epoch 2
+                0,
+                4
+            )),
+            calculate_stake_rewards(
+                2,
+                &stake,
+                &PointValue {
+                    rewards: 1,
+                    points: 1
+                },
+                &vote_state,
+                None,
+                &mut null_tracer(),
+                true,
+            )
+        );
+
+        // credits_observed is moved forward for the stake's activation epoch,
+        // and no rewards are perceived
+        stake.delegation.activation_epoch = 2;
+        stake.credits_observed = 3;
+        assert_eq!(
+            Some((0, 0, 4)),
+            calculate_stake_rewards(
+                2,
+                &stake,
+                &PointValue {
+                    rewards: 1,
+                    points: 1
+                },
+                &vote_state,
+                None,
+                &mut null_tracer(),
+                true,
+            )
+        );
     }
 
     #[test]

runtime/src/bank.rs

@@ -1940,8 +1940,12 @@ impl Bank {
 
         let old_vote_balance_and_staked = self.stakes.read().unwrap().vote_balance_and_staked();
 
-        let validator_point_value =
+        let validator_point_value = self.pay_validator_rewards(
-            self.pay_validator_rewards(prev_epoch, validator_rewards, reward_calc_tracer);
+            prev_epoch,
+            validator_rewards,
+            reward_calc_tracer,
+            self.stake_program_advance_activating_credits_observed(),
+        );
 
         if !self
             .feature_set
@@ -2078,6 +2082,7 @@ impl Bank {
         rewarded_epoch: Epoch,
         rewards: u64,
         reward_calc_tracer: &mut Option<impl FnMut(&RewardCalculationEvent)>,
+        fix_activating_credits_observed: bool,
     ) -> f64 {
         let stake_history = self.stakes.read().unwrap().history().clone();
 
@@ -2136,6 +2141,7 @@ impl Bank {
                     &point_value,
                     Some(&stake_history),
                     &mut reward_calc_tracer.as_mut(),
+                    fix_activating_credits_observed,
                 );
                 if let Ok((stakers_reward, _voters_reward)) = redeemed {
                     self.store_account(stake_pubkey, stake_account);
@@ -5365,6 +5371,11 @@ impl Bank {
             .is_active(&feature_set::versioned_tx_message_enabled::id())
     }
 
+    pub fn stake_program_advance_activating_credits_observed(&self) -> bool {
+        self.feature_set
+            .is_active(&feature_set::stake_program_advance_activating_credits_observed::id())
+    }
+
     // Check if the wallclock time from bank creation to now has exceeded the allotted
     // time for transaction processing
     pub fn should_bank_still_be_processing_txs(

sdk/src/feature_set.rs

@@ -187,6 +187,10 @@ pub mod close_upgradeable_program_accounts {
     solana_sdk::declare_id!("EQMtCuSAkMVF9ZdhGuABtgvyXJLtSRF5AQKv1RNsrhj7");
 }
 
+pub mod stake_program_advance_activating_credits_observed {
+    solana_sdk::declare_id!("SAdVFw3RZvzbo6DvySbSdBnHN4gkzSTH9dSxesyKKPj");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -229,6 +233,7 @@ lazy_static! {
         (libsecp256k1_fail_on_bad_count::id(), "Fail libsec256k1_verify if count appears wrong"),
         (instructions_sysvar_owned_by_sysvar::id(), "fix owner for instructions sysvar"),
         (close_upgradeable_program_accounts::id(), "enable closing upgradeable program accounts"),
+        (stake_program_advance_activating_credits_observed::id(), "Enable advancing credits observed for activation epoch #19309"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()