Credits auto rewind on vote recreation (#22546)
* Credits auto rewind on vote recreation * Update comment * Improve comments and tests * Recommended fn rename * Restore old feature, and replace new feature key Co-authored-by: Tyera Eulberg <tyera@solana.com>
This commit is contained in:
parent
f244a2e141
commit
412a5a0d33
|
@ -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,
|
||||
|
|
|
@ -43,6 +43,7 @@ pub enum SkippedReason {
|
|||
ZeroReward,
|
||||
ZeroCreditsAndReturnZero,
|
||||
ZeroCreditsAndReturnCurrent,
|
||||
ZeroCreditsAndReturnRewinded,
|
||||
}
|
||||
|
||||
impl From<SkippedReason> for InflationPointCalculationEvent {
|
||||
|
@ -164,7 +165,7 @@ fn redeem_stake_rewards(
|
|||
vote_state: &VoteState,
|
||||
stake_history: Option<&StakeHistory>,
|
||||
inflation_point_calc_tracer: Option<impl Fn(&InflationPointCalculationEvent)>,
|
||||
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<impl Fn(&InflationPointCalculationEvent)>,
|
||||
) -> (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<impl Fn(&InflationPointCalculationEvent)>,
|
||||
_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<impl Fn(&InflationPointCalculationEvent)>,
|
||||
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
|
||||
|
|
|
@ -2637,7 +2637,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,
|
||||
|
@ -2971,7 +2971,7 @@ impl Bank {
|
|||
rewarded_epoch: Epoch,
|
||||
rewards: u64,
|
||||
reward_calc_tracer: Option<impl Fn(&RewardCalculationEvent) + Send + Sync>,
|
||||
fix_activating_credits_observed: bool,
|
||||
credits_auto_rewind: bool,
|
||||
thread_pool: &ThreadPool,
|
||||
metrics: &mut RewardsMetrics,
|
||||
update_rewards_from_cached_accounts: bool,
|
||||
|
@ -3087,7 +3087,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
|
||||
|
@ -3327,6 +3327,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
|
||||
}
|
||||
|
@ -6743,9 +6748,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 {
|
||||
|
|
|
@ -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"),
|
||||
|
|
Loading…
Reference in New Issue