diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 11b65c7f2fd..51cfa29b325 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -52,6 +52,8 @@ use { account::{AccountSharedData, ReadableAccount, WritableAccount}, account_utils::StateMut, clock::{Epoch, Slot}, + feature::{self, Feature}, + feature_set, genesis_config::{ClusterType, GenesisConfig}, hash::Hash, inflation::Inflation, @@ -1537,6 +1539,13 @@ fn main() { .possible_values(&["pico", "full", "none"]) .help("Overwrite inflation when warping"), ) + .arg( + Arg::with_name("enable_credits_auto_rewind") + .required(false) + .long("enable-credits-auto-rewind") + .takes_value(false) + .help("Enable credits auto rewind"), + ) .arg( Arg::with_name("recalculate_capitalization") .required(false) @@ -2734,6 +2743,66 @@ fn main() { .lazy_rent_collection .store(true, std::sync::atomic::Ordering::Relaxed); + let feature_account_balance = std::cmp::max( + genesis_config.rent.minimum_balance(Feature::size_of()), + 1, + ); + if arg_matches.is_present("enable_credits_auto_rewind") { + base_bank.unfreeze_for_ledger_tool(); + let mut force_enabled_count = 0; + if base_bank + .get_account(&feature_set::credits_auto_rewind::id()) + .is_none() + { + base_bank.store_account( + &feature_set::credits_auto_rewind::id(), + &feature::create_account( + &Feature { activated_at: None }, + feature_account_balance, + ), + ); + force_enabled_count += 1; + } + if force_enabled_count == 0 { + warn!( + "Already credits_auto_rewind is activated (or scheduled)" + ); + } + let mut store_failed_count = 0; + if force_enabled_count >= 1 { + if base_bank + .get_account(&feature_set::deprecate_rewards_sysvar::id()) + .is_some() + { + // steal some lamports from the pretty old feature not to affect + // capitalizaion, which doesn't affect inflation behavior! + base_bank.store_account( + &feature_set::deprecate_rewards_sysvar::id(), + &AccountSharedData::default(), + ); + force_enabled_count -= 1; + } else { + store_failed_count += 1; + } + } + assert_eq!(force_enabled_count, store_failed_count); + if store_failed_count >= 1 { + // we have no choice; maybe locally created blank cluster with + // not-Development cluster type. + let old_cap = base_bank.set_capitalization(); + let new_cap = base_bank.capitalization(); + warn!( + "Skewing capitalization a bit to enable credits_auto_rewind as \ + requested: increasing {} from {} to {}", + feature_account_balance, old_cap, new_cap, + ); + assert_eq!( + old_cap + feature_account_balance * store_failed_count, + new_cap + ); + } + } + #[derive(Default, Debug)] struct PointDetail { epoch: Epoch, diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index 7cc4cc16c07..53a4a684211 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -43,6 +43,7 @@ pub enum SkippedReason { ZeroReward, ZeroCreditsAndReturnZero, ZeroCreditsAndReturnCurrent, + ZeroCreditsAndReturnRewinded, } impl From for InflationPointCalculationEvent { @@ -164,7 +165,7 @@ fn redeem_stake_rewards( vote_state: &VoteState, stake_history: Option<&StakeHistory>, inflation_point_calc_tracer: Option, - fix_activating_credits_observed: bool, + credits_auto_rewind: bool, ) -> Option<(u64, u64)> { if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { inflation_point_calc_tracer(&InflationPointCalculationEvent::CreditsObserved( @@ -179,7 +180,7 @@ fn redeem_stake_rewards( vote_state, stake_history, inflation_point_calc_tracer.as_ref(), - fix_activating_credits_observed, + credits_auto_rewind, ) .map(|(stakers_reward, voters_reward, credits_observed)| { if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer { @@ -205,6 +206,8 @@ 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 ) .0 } @@ -217,15 +220,47 @@ fn calculate_stake_points_and_credits( new_vote_state: &VoteState, stake_history: Option<&StakeHistory>, inflation_point_calc_tracer: Option, -) -> (u128, u64) { + credits_auto_rewind: bool, +) -> (u128, u64, bool) { 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 if credits_in_vote <= credits_in_stake { - if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { - inflation_point_calc_tracer(&SkippedReason::ZeroCreditsAndReturnCurrent.into()); - } - return (0, credits_in_stake); + if credits_auto_rewind && credits_in_vote < credits_in_stake { + if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { + inflation_point_calc_tracer(&SkippedReason::ZeroCreditsAndReturnRewinded.into()); + } + // Don't adjust stake.activation_epoch for simplicity: + // - generally fast-forwarding stake.activation_epoch forcibly (for + // artifical re-activation with re-warm-up) skews the stake + // history sysvar. And properly handling all the cases + // regarding deactivation epoch/warm-up/cool-down without + // introducing incentive skew is hard. + // - Conceptually, it should be acceptable for the staked SOLs at + // the recreated vote to receive rewards again immediately after + // rewind even if it looks like instant activation. That's + // because it must have passed the required warmed-up at least + // once in the past already + // - Also such a stake account remains to be a part of overall + // effective stake calculation even while the vote account is + // missing for (indefinite) time or remains to be pre-remove + // credits score. It should be treated equally to staking with + // delinquent validator with no differenciation. + + // hint with true to indicate some exceptional credits handling is needed + return (0, credits_in_vote, 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 + // after activation + + if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { + inflation_point_calc_tracer(&SkippedReason::ZeroCreditsAndReturnCurrent.into()); + } + // 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); + }; } let mut points = 0; @@ -268,7 +303,7 @@ fn calculate_stake_points_and_credits( } } - (points, new_credits_observed) + (points, new_credits_observed, false) } /// for a given stake and vote_state, calculate what distributions and what updates should be made @@ -284,17 +319,17 @@ fn calculate_stake_rewards( vote_state: &VoteState, stake_history: Option<&StakeHistory>, inflation_point_calc_tracer: Option, - _fix_activating_credits_observed: bool, // this unused flag will soon be justified by an upcoming feature pr + credits_auto_rewind: bool, ) -> Option<(u64, u64, u64)> { // ensure to run to trigger (optional) inflation_point_calc_tracer - // this awkward flag variable will soon be justified by an upcoming feature pr - let mut forced_credits_update_with_skipped_reward = false; - let (points, credits_observed) = calculate_stake_points_and_credits( - stake, - vote_state, - stake_history, - inflation_point_calc_tracer.as_ref(), - ); + 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, + ); // Drive credits_observed forward unconditionally when rewards are disabled // or when this is the stake's activation epoch @@ -1302,7 +1337,7 @@ pub fn redeem_rewards( point_value: &PointValue, stake_history: Option<&StakeHistory>, inflation_point_calc_tracer: Option, - fix_activating_credits_observed: bool, + credits_auto_rewind: bool, ) -> Result<(u64, u64), InstructionError> { if let StakeState::Stake(meta, mut stake) = stake_state { if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { @@ -1326,7 +1361,7 @@ pub fn redeem_rewards( vote_state, stake_history, inflation_point_calc_tracer, - fix_activating_credits_observed, + credits_auto_rewind, ) { stake_account.checked_add_lamports(stakers_reward)?; stake_account.set_state(&StakeState::Stake(meta, stake))?; @@ -2546,10 +2581,29 @@ mod tests { ) ); - // assert the previous behavior is preserved where fix_stake_deactivate=false assert_eq!( - (0, 4), - calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer()) + (0, 4, false), + calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), true) + ); + + // credits_observed is auto-rewinded when vote_state credits are assumed to have been + // recreated + stake.credits_observed = 1000; + // this is old behavior; return the pre-recreation (large) credits from stake account + assert_eq!( + (0, 1000, 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), + 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), + calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer(), true) ); // get rewards and credits observed when not the activation epoch diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e2001aa3fce..ecc6c7afee5 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -2634,7 +2634,7 @@ impl Bank { prev_epoch, validator_rewards, reward_calc_tracer, - self.stake_program_advance_activating_credits_observed(), + self.credits_auto_rewind(), thread_pool, metrics, update_rewards_from_cached_accounts, @@ -2968,7 +2968,7 @@ impl Bank { rewarded_epoch: Epoch, rewards: u64, reward_calc_tracer: Option, - fix_activating_credits_observed: bool, + credits_auto_rewind: bool, thread_pool: &ThreadPool, metrics: &mut RewardsMetrics, update_rewards_from_cached_accounts: bool, @@ -3084,7 +3084,7 @@ impl Bank { &point_value, Some(&stake_history), reward_calc_tracer.as_ref(), - fix_activating_credits_observed, + credits_auto_rewind, ); if let Ok((stakers_reward, voters_reward)) = redeemed { // track voter rewards @@ -3324,6 +3324,11 @@ impl Bank { } } + // dangerous; don't use this; this is only needed for ledger-tool's special command + pub fn unfreeze_for_ledger_tool(&self) { + self.freeze_started.store(false, Relaxed); + } + pub fn epoch_schedule(&self) -> &EpochSchedule { &self.epoch_schedule } @@ -6718,9 +6723,9 @@ impl Bank { .is_active(&feature_set::versioned_tx_message_enabled::id()) } - pub fn stake_program_advance_activating_credits_observed(&self) -> bool { + pub fn credits_auto_rewind(&self) -> bool { self.feature_set - .is_active(&feature_set::stake_program_advance_activating_credits_observed::id()) + .is_active(&feature_set::credits_auto_rewind::id()) } pub fn leave_nonce_on_success(&self) -> bool { diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index ba829dc4d58..dda51d819d0 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -165,6 +165,10 @@ pub mod stake_program_advance_activating_credits_observed { solana_sdk::declare_id!("SAdVFw3RZvzbo6DvySbSdBnHN4gkzSTH9dSxesyKKPj"); } +pub mod credits_auto_rewind { + solana_sdk::declare_id!("BUS12ciZ5gCoFafUHWW8qaFMMtwFQGVxjsDheWLdqBE2"); +} + pub mod demote_program_write_locks { solana_sdk::declare_id!("3E3jV7v9VcdJL8iYZUMax9DiDno8j7EWUVbhm9RtShj2"); } @@ -399,6 +403,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"), (stake_program_advance_activating_credits_observed::id(), "Enable advancing credits observed for activation epoch #19309"), + (credits_auto_rewind::id(), "Auto rewind stake's credits_observed if (accidental) vote recreation is detected #22546"), (demote_program_write_locks::id(), "demote program write locks to readonly, except when upgradeable loader present #19593 #20265"), (ed25519_program_enabled::id(), "enable builtin ed25519 signature verify program"), (return_data_syscall_enabled::id(), "enable sol_{set,get}_return_data syscall"),