Improve metrics/logging; also skip unchanged accts

programs/stake/src/stake_state.rs

@@ -167,13 +167,21 @@ impl Meta {
         Ok(())
     }
 
-    pub fn reduce_overage_rent_exempt_reserve(&mut self, rent: &Rent, data_len: usize) {
-        let rent_exempt_reserve = rent.minimum_balance(data_len);
-        if rent_exempt_reserve != self.rent_exempt_reserve {
+    pub fn reduce_overage_rent_exempt_reserve(
+        &mut self,
+        rent: &Rent,
+        data_len: usize,
+    ) -> Option<(u64, u64)> {
+        let corrected_rent_exempt_reserve = rent.minimum_balance(data_len);
+        if corrected_rent_exempt_reserve != self.rent_exempt_reserve {
             // we forcibly update rent_excempt_reserve even if rent_exempt_reserve > account
             // balance, hoping user might restore rent_exempt status by depositing.
             // TODO: split is overflow safe?
-            self.rent_exempt_reserve = rent_exempt_reserve;
+            let (old, new) = (self.rent_exempt_reserve, corrected_rent_exempt_reserve);
+            self.rent_exempt_reserve = corrected_rent_exempt_reserve;
+            Some((old, new))
+        } else {
+            None
         }
     }
 }
@@ -698,13 +706,18 @@ impl Stake {
         }
     }
 
-    fn reduce_overage(&mut self, balance: u64, rent_exempt_balance: u64) {
+    fn reduce_overage(&mut self, balance: u64, rent_exempt_balance: u64) -> Option<(u64, u64)> {
         // at this moment, balance >>> rent_excempt_balance (no overflow risk)
         //   => no there could be possibility of non-rent-exempt stake accounts
+        // this will overwrite innocent deactivating and immeditealy withdrawn stake accounts as well is this ok?
         let corrected_stake = balance.saturating_sub(rent_exempt_balance);
         if self.delegation.stake > corrected_stake {
             // this could result in creating 0 stake account..; is this safe?
+            let (old, new) = (self.delegation.stake, corrected_stake);
             self.delegation.stake = corrected_stake;
+            Some((old, new))
+        } else {
+            None
         }
     }
 }
@@ -1192,17 +1205,42 @@ fn calculate_split_rent_exempt_reserve(
     lamports_per_byte_year * (split_data_len + ACCOUNT_STORAGE_OVERHEAD)
 }
 
+pub type ReducedOverageStakeResult = (&'static str, (u64, u64), (u64, u64));
+
 pub fn reduce_overage_stakes(
     stake_account: &mut Account,
     rent: &Rent,
-) -> Result<(), InstructionError> {
-    if let StakeState::Stake(mut meta, mut stake) = stake_account.state()? {
-        meta.reduce_overage_rent_exempt_reserve(rent, stake_account.data.len());
-        stake.reduce_overage(stake_account.lamports, meta.rent_exempt_reserve);
-        stake_account.set_state(&StakeState::Stake(meta, stake))
-    // match StakeState::Initialized? to adjust rent_excempt reserve? or prove no need to do it?
-    } else {
-        Err(InstructionError::InvalidAccountData)
+) -> Result<ReducedOverageStakeResult, InstructionError> {
+    match stake_account.state()? {
+        StakeState::Initialized(mut meta) => {
+            let meta_result =
+                meta.reduce_overage_rent_exempt_reserve(rent, stake_account.data.len());
+            stake_account.set_state(&StakeState::Initialized(meta))?;
+
+            if meta_result.is_none() {
+                Err(InstructionError::InvalidAccountData)
+            } else {
+                Ok(("initialized", meta_result.unwrap_or_default(), (0, 0)))
+            }
+        }
+        StakeState::Stake(mut meta, mut stake) => {
+            let meta_result =
+                meta.reduce_overage_rent_exempt_reserve(rent, stake_account.data.len());
+            let stake_result =
+                stake.reduce_overage(stake_account.lamports, meta.rent_exempt_reserve);
+            stake_account.set_state(&StakeState::Stake(meta, stake))?;
+
+            if meta_result.is_none() && stake_result.is_none() {
+                Err(InstructionError::InvalidAccountData)
+            } else {
+                Ok((
+                    "stake",
+                    meta_result.unwrap_or_default(),
+                    stake_result.unwrap_or_default(),
+                ))
+            }
+        }
+        _ => Err(InstructionError::InvalidAccountData),
     }
 }
 

runtime/src/bank.rs

@@ -1336,21 +1336,37 @@ impl Bank {
 
     fn reduce_overage_stakes(&self) {
         // is this really all of parsable accounts owned by stake11111?
+        let mut count = 0;
         self.cloned_stake_delegations()
             .into_iter()
             .for_each(|(stake_pubkey, _delegation)| {
                 if let Some(mut stake_account) = self.get_account(&stake_pubkey) {
                     // metrics to record occurances??
-                    if stake_state::reduce_overage_stakes(
+                    if let Ok(result) = stake_state::reduce_overage_stakes(
                         &mut stake_account,
                         &self.rent_collector.rent,
-                    )
-                    .is_ok()
-                    {
+                    ) {
                         self.store_account(&stake_pubkey, &stake_account);
+                        let message =
+                            format!("reduced overage stake: {}, {:?}", stake_pubkey, result,);
+                        info!("{}", message);
+                        datapoint_info!("stake_info", ("info", message, String));
+                        count += 1;
                     }
                 }
             });
+
+        let msg = format!(
+            "bank (slot: {}): reduce_overage_stakes: {} accounts rewritten",
+            self.slot(),
+            count
+        );
+        if self.cluster_type() != ClusterType::Development {
+            info!("{}", msg);
+        } else {
+            // don't spam log; this is called at every slot for testing only on development clusters
+            trace!("{}", msg);
+        }
     }
 
     // update rewards based on the previous epoch