removes feature-gate code for updating rewards from cached accounts (#32514)

This commit is contained in:
behzad nouri 2023-07-21 19:52:44 +00:00 committed by GitHub
parent 9e8639f7ae
commit 952d8861c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 10 additions and 278 deletions

View File

@ -476,7 +476,6 @@ pub const ACCOUNTS_DB_CONFIG_FOR_TESTING: AccountsDbConfig = AccountsDbConfig {
ancient_append_vec_offset: None,
skip_initial_hash_calc: false,
exhaustively_verify_refcounts: false,
assert_stakes_cache_consistency: true,
create_ancient_storage: CreateAncientStorage::Pack,
test_partitioned_epoch_rewards: TestPartitionedEpochRewards::CompareResults,
};
@ -488,7 +487,6 @@ pub const ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS: AccountsDbConfig = AccountsDbConfig
ancient_append_vec_offset: None,
skip_initial_hash_calc: false,
exhaustively_verify_refcounts: false,
assert_stakes_cache_consistency: false,
create_ancient_storage: CreateAncientStorage::Pack,
test_partitioned_epoch_rewards: TestPartitionedEpochRewards::None,
};
@ -550,8 +548,6 @@ pub struct AccountsDbConfig {
pub ancient_append_vec_offset: Option<i64>,
pub skip_initial_hash_calc: bool,
pub exhaustively_verify_refcounts: bool,
/// when stakes cache consistency check occurs, assert that cached accounts match accounts db
pub assert_stakes_cache_consistency: bool,
/// how to create ancient storages
pub create_ancient_storage: CreateAncientStorage,
pub test_partitioned_epoch_rewards: TestPartitionedEpochRewards,
@ -1378,9 +1374,6 @@ pub struct AccountsDb {
pub(crate) storage: AccountStorage,
/// from AccountsDbConfig
pub(crate) assert_stakes_cache_consistency: bool,
#[allow(dead_code)]
/// from AccountsDbConfig
create_ancient_storage: CreateAncientStorage,
@ -2390,7 +2383,6 @@ impl AccountsDb {
const ACCOUNTS_STACK_SIZE: usize = 8 * 1024 * 1024;
AccountsDb {
assert_stakes_cache_consistency: false,
bank_progress: BankCreationFreezingProgress::default(),
create_ancient_storage: CreateAncientStorage::Pack,
verify_accounts_hash_in_bg: VerifyAccountsHashInBackground::default(),
@ -2515,11 +2507,6 @@ impl AccountsDb {
.map(|config| config.exhaustively_verify_refcounts)
.unwrap_or_default();
let assert_stakes_cache_consistency = accounts_db_config
.as_ref()
.map(|config| config.assert_stakes_cache_consistency)
.unwrap_or_default();
let create_ancient_storage = accounts_db_config
.as_ref()
.map(|config| config.create_ancient_storage)
@ -2549,7 +2536,6 @@ impl AccountsDb {
accounts_update_notifier,
filler_accounts_config,
filler_account_suffix,
assert_stakes_cache_consistency,
create_ancient_storage,
write_cache_limit_bytes: accounts_db_config
.as_ref()

View File

@ -68,7 +68,7 @@ use {
serde_snapshot::BankIncrementalSnapshotPersistence,
snapshot_hash::SnapshotHash,
sorted_storages::SortedStorages,
stake_account::{self, StakeAccount},
stake_account::StakeAccount,
stake_history::StakeHistory,
stake_rewards::StakeReward,
stake_weighted_timestamp::{
@ -174,7 +174,7 @@ use {
self, InflationPointCalculationEvent, PointValue, StakeState,
},
solana_system_program::{get_system_account_kind, SystemAccountKind},
solana_vote_program::vote_state::{VoteState, VoteStateVersions},
solana_vote_program::vote_state::VoteState,
std::{
borrow::Cow,
cell::RefCell,
@ -819,8 +819,7 @@ pub struct Bank {
struct VoteWithStakeDelegations {
vote_state: Arc<VoteState>,
vote_account: AccountSharedData,
// TODO: use StakeAccount<Delegation> once the old code is deleted.
delegations: Vec<(Pubkey, StakeAccount<()>)>,
delegations: Vec<(Pubkey, StakeAccount<Delegation>)>,
}
type VoteWithStakeDelegationsMap = DashMap<Pubkey, VoteWithStakeDelegations>;
@ -829,11 +828,7 @@ type InvalidCacheKeyMap = DashMap<Pubkey, InvalidCacheEntryReason>;
struct LoadVoteAndStakeAccountsResult {
vote_with_stake_delegations_map: VoteWithStakeDelegationsMap,
invalid_stake_keys: InvalidCacheKeyMap,
invalid_vote_keys: InvalidCacheKeyMap,
invalid_cached_vote_accounts: usize,
invalid_cached_stake_accounts: usize,
invalid_cached_stake_accounts_rent_epoch: usize,
vote_accounts_cache_miss_count: usize,
}
@ -2547,9 +2542,6 @@ impl Bank {
} = self.calculate_previous_epoch_inflation_rewards(capitalization, prev_epoch);
let old_vote_balance_and_staked = self.stakes_cache.stakes().vote_balance_and_staked();
let update_rewards_from_cached_accounts = self
.feature_set
.is_active(&feature_set::update_rewards_from_cached_accounts::id());
self.pay_validator_rewards_with_thread_pool(
prev_epoch,
@ -2557,7 +2549,6 @@ impl Bank {
reward_calc_tracer,
thread_pool,
metrics,
update_rewards_from_cached_accounts,
);
let new_vote_balance_and_staked = self.stakes_cache.stakes().vote_balance_and_staked();
@ -2621,167 +2612,6 @@ impl Bank {
);
}
/// map stake delegations into resolved (pubkey, account) pairs
/// returns a map (has to be copied) of loaded
/// ( Vec<(staker info)> (voter account) ) keyed by voter pubkey
///
/// Filters out invalid pairs
fn _load_vote_and_stake_accounts_with_thread_pool(
&self,
thread_pool: &ThreadPool,
reward_calc_tracer: Option<impl RewardCalcTracer>,
) -> LoadVoteAndStakeAccountsResult {
let stakes = self.stakes_cache.stakes();
let cached_vote_accounts = stakes.vote_accounts();
let vote_with_stake_delegations_map = DashMap::with_capacity(cached_vote_accounts.len());
let invalid_stake_keys: DashMap<Pubkey, InvalidCacheEntryReason> = DashMap::new();
let invalid_vote_keys: DashMap<Pubkey, InvalidCacheEntryReason> = DashMap::new();
let invalid_cached_stake_accounts = AtomicUsize::default();
let invalid_cached_vote_accounts = AtomicUsize::default();
let invalid_cached_stake_accounts_rent_epoch = AtomicUsize::default();
let stake_delegations = self.filter_stake_delegations(&stakes);
thread_pool.install(|| {
stake_delegations
.into_par_iter()
.for_each(|(stake_pubkey, cached_stake_account)| {
let delegation = cached_stake_account.delegation();
let vote_pubkey = &delegation.voter_pubkey;
if invalid_vote_keys.contains_key(vote_pubkey) {
return;
}
let Some(stake_account) = self.get_account_with_fixed_root(stake_pubkey) else {
invalid_stake_keys
.insert(*stake_pubkey, InvalidCacheEntryReason::Missing);
invalid_cached_stake_accounts.fetch_add(1, Relaxed);
return;
};
if cached_stake_account.account() != &stake_account {
if self.rc.accounts.accounts_db.assert_stakes_cache_consistency {
panic!(
"stakes cache accounts mismatch {cached_stake_account:?} {stake_account:?}"
);
}
invalid_cached_stake_accounts.fetch_add(1, Relaxed);
let cached_stake_account = cached_stake_account.account();
if cached_stake_account.lamports() == stake_account.lamports()
&& cached_stake_account.data() == stake_account.data()
&& cached_stake_account.owner() == stake_account.owner()
&& cached_stake_account.executable() == stake_account.executable()
{
invalid_cached_stake_accounts_rent_epoch.fetch_add(1, Relaxed);
} else {
debug!(
"cached stake account mismatch: {}: {:?}, {:?}",
stake_pubkey, stake_account, cached_stake_account
);
}
}
let stake_account = match StakeAccount::<()>::try_from(stake_account) {
Ok(stake_account) => stake_account,
Err(stake_account::Error::InvalidOwner { .. }) => {
invalid_stake_keys
.insert(*stake_pubkey, InvalidCacheEntryReason::WrongOwner);
return;
}
Err(stake_account::Error::InstructionError(_)) => {
invalid_stake_keys
.insert(*stake_pubkey, InvalidCacheEntryReason::BadState);
return;
}
Err(stake_account::Error::InvalidDelegation(_)) => {
// This should not happen.
error!(
"Unexpected code path! StakeAccount<()> \
should not check if stake-state is a \
Delegation."
);
return;
}
};
let stake_delegation = (*stake_pubkey, stake_account);
let mut vote_delegations = if let Some(vote_delegations) =
vote_with_stake_delegations_map.get_mut(vote_pubkey)
{
vote_delegations
} else {
let cached_vote_account = cached_vote_accounts.get(vote_pubkey);
let vote_account = match self.get_account_with_fixed_root(vote_pubkey) {
Some(vote_account) => {
if vote_account.owner() != &solana_vote_program::id() {
invalid_vote_keys
.insert(*vote_pubkey, InvalidCacheEntryReason::WrongOwner);
if cached_vote_account.is_some() {
invalid_cached_vote_accounts.fetch_add(1, Relaxed);
}
return;
}
vote_account
}
None => {
if cached_vote_account.is_some() {
invalid_cached_vote_accounts.fetch_add(1, Relaxed);
}
invalid_vote_keys
.insert(*vote_pubkey, InvalidCacheEntryReason::Missing);
return;
}
};
let vote_state = if let Ok(vote_state) =
StateMut::<VoteStateVersions>::state(&vote_account)
{
vote_state.convert_to_current()
} else {
invalid_vote_keys
.insert(*vote_pubkey, InvalidCacheEntryReason::BadState);
if cached_vote_account.is_some() {
invalid_cached_vote_accounts.fetch_add(1, Relaxed);
}
return;
};
match cached_vote_account {
Some(cached_vote_account)
if cached_vote_account.account() == &vote_account => {}
_ => {
invalid_cached_vote_accounts.fetch_add(1, Relaxed);
}
};
vote_with_stake_delegations_map
.entry(*vote_pubkey)
.or_insert_with(|| VoteWithStakeDelegations {
vote_state: Arc::new(vote_state),
vote_account,
delegations: vec![],
})
};
if let Some(reward_calc_tracer) = reward_calc_tracer.as_ref() {
reward_calc_tracer(&RewardCalculationEvent::Staking(
stake_pubkey,
&InflationPointCalculationEvent::Delegation(
delegation,
solana_vote_program::id(),
),
));
}
vote_delegations.delegations.push(stake_delegation);
});
});
invalid_cached_stake_accounts.fetch_add(invalid_stake_keys.len(), Relaxed);
LoadVoteAndStakeAccountsResult {
vote_with_stake_delegations_map,
invalid_vote_keys,
invalid_stake_keys,
invalid_cached_vote_accounts: invalid_cached_vote_accounts.into_inner(),
invalid_cached_stake_accounts: invalid_cached_stake_accounts.into_inner(),
invalid_cached_stake_accounts_rent_epoch: invalid_cached_stake_accounts_rent_epoch
.into_inner(),
vote_accounts_cache_miss_count: 0,
}
}
fn filter_stake_delegations<'a>(
&self,
stakes: &'a Stakes<StakeAccount<Delegation>>,
@ -2906,8 +2736,7 @@ impl Bank {
let event = RewardCalculationEvent::Staking(stake_pubkey, &delegation);
reward_calc_tracer(&event);
}
let stake_account = StakeAccount::from(stake_account.clone());
let stake_delegation = (*stake_pubkey, stake_account);
let stake_delegation = (*stake_pubkey, stake_account.clone());
vote_delegations.delegations.push(stake_delegation);
};
thread_pool.install(|| {
@ -2918,10 +2747,6 @@ impl Bank {
LoadVoteAndStakeAccountsResult {
vote_with_stake_delegations_map,
invalid_vote_keys,
invalid_stake_keys: DashMap::default(),
invalid_cached_vote_accounts: 0,
invalid_cached_stake_accounts: 0,
invalid_cached_stake_accounts_rent_epoch: 0,
vote_accounts_cache_miss_count: vote_accounts_cache_miss_count.into_inner(),
}
}
@ -2982,15 +2807,10 @@ impl Bank {
reward_calc_tracer: Option<impl RewardCalcTracer>,
thread_pool: &ThreadPool,
metrics: &mut RewardsMetrics,
update_rewards_from_cached_accounts: bool,
) {
let stake_history = self.stakes_cache.stakes().history().clone();
let vote_with_stake_delegations_map = self.load_vote_and_stake_accounts(
thread_pool,
reward_calc_tracer.as_ref(),
metrics,
update_rewards_from_cached_accounts,
);
let vote_with_stake_delegations_map =
self.load_vote_and_stake_accounts(thread_pool, reward_calc_tracer.as_ref(), metrics);
let point_value = self.calculate_reward_points(
&vote_with_stake_delegations_map,
@ -3130,39 +2950,23 @@ impl Bank {
thread_pool: &ThreadPool,
reward_calc_tracer: Option<impl RewardCalcTracer>,
metrics: &mut RewardsMetrics,
update_rewards_from_cached_accounts: bool,
) -> VoteWithStakeDelegationsMap {
let (
LoadVoteAndStakeAccountsResult {
vote_with_stake_delegations_map,
invalid_stake_keys,
invalid_vote_keys,
invalid_cached_vote_accounts,
invalid_cached_stake_accounts,
invalid_cached_stake_accounts_rent_epoch,
vote_accounts_cache_miss_count,
},
measure,
) = measure!({
if update_rewards_from_cached_accounts {
self._load_vote_and_stake_accounts(thread_pool, reward_calc_tracer.as_ref())
} else {
self._load_vote_and_stake_accounts_with_thread_pool(
thread_pool,
reward_calc_tracer.as_ref(),
)
}
self._load_vote_and_stake_accounts(thread_pool, reward_calc_tracer.as_ref())
});
metrics
.load_vote_and_stake_accounts_us
.fetch_add(measure.as_us(), Relaxed);
metrics.invalid_cached_vote_accounts += invalid_cached_vote_accounts;
metrics.invalid_cached_stake_accounts += invalid_cached_stake_accounts;
metrics.invalid_cached_stake_accounts_rent_epoch +=
invalid_cached_stake_accounts_rent_epoch;
metrics.vote_accounts_cache_miss_count += vote_accounts_cache_miss_count;
self.stakes_cache
.handle_invalid_keys(invalid_stake_keys, invalid_vote_keys, self.slot());
.handle_invalid_keys(invalid_vote_keys, self.slot());
vote_with_stake_delegations_map
}

View File

@ -19,9 +19,6 @@ pub(crate) struct RewardsMetrics {
pub(crate) redeem_rewards_us: u64,
pub(crate) store_stake_accounts_us: AtomicU64,
pub(crate) store_vote_accounts_us: AtomicU64,
pub(crate) invalid_cached_vote_accounts: usize,
pub(crate) invalid_cached_stake_accounts: usize,
pub(crate) invalid_cached_stake_accounts_rent_epoch: usize,
pub(crate) vote_accounts_cache_miss_count: usize,
pub(crate) hash_partition_rewards_us: u64,
}
@ -96,21 +93,6 @@ pub(crate) fn report_new_epoch_metrics(
metrics.store_vote_accounts_us.load(Relaxed),
i64
),
(
"invalid_cached_vote_accounts",
metrics.invalid_cached_vote_accounts,
i64
),
(
"invalid_cached_stake_accounts",
metrics.invalid_cached_stake_accounts,
i64
),
(
"invalid_cached_stake_accounts_rent_epoch",
metrics.invalid_cached_stake_accounts_rent_epoch,
i64
),
(
"vote_accounts_cache_miss_count",
metrics.vote_accounts_cache_miss_count,

View File

@ -2071,9 +2071,6 @@ fn test_rent_eager_collect_rent_zero_lamport_deterministic() {
#[test]
fn test_bank_update_vote_stake_rewards() {
let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap();
check_bank_update_vote_stake_rewards(|bank: &Bank| {
bank._load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
});
check_bank_update_vote_stake_rewards(|bank: &Bank| {
bank._load_vote_and_stake_accounts(&thread_pool, null_tracer())
});
@ -9143,12 +9140,6 @@ fn test_get_inflation_num_slots_already_activated() {
#[test]
fn test_stake_vote_account_validity() {
let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap();
check_stake_vote_account_validity(
true, // check owner change,
|bank: &Bank| {
bank._load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
},
);
// TODO: stakes cache should be hardened for the case when the account
// owner is changed from vote/stake program to something else. see:
// https://github.com/solana-labs/solana/pull/24200#discussion_r849935444
@ -9179,11 +9170,7 @@ where
AccountSecondaryIndexes::default(),
AccountShrinkThreshold::default(),
false,
Some(AccountsDbConfig {
// at least one tests hit this assert, so disable it
assert_stakes_cache_consistency: false,
..ACCOUNTS_DB_CONFIG_FOR_TESTING
}),
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
None,
Arc::default(),
));
@ -11953,7 +11940,6 @@ fn test_stake_account_consistency_with_rent_epoch_max_feature(
Epoch::default()
};
assert!(bank.rc.accounts.accounts_db.assert_stakes_cache_consistency);
let mut pubkey_bytes_early = [0u8; 32];
pubkey_bytes_early[31] = 2;
let stake_id1 = Pubkey::from(pubkey_bytes_early);

View File

@ -44,10 +44,6 @@ impl<T> StakeAccount<T> {
pub(crate) fn stake_state(&self) -> &StakeState {
&self.stake_state
}
pub(crate) fn account(&self) -> &AccountSharedData {
&self.account
}
}
impl StakeAccount<Delegation> {
@ -91,17 +87,6 @@ impl TryFrom<AccountSharedData> for StakeAccount<Delegation> {
}
}
impl From<StakeAccount<Delegation>> for StakeAccount<()> {
#[inline]
fn from(stake_account: StakeAccount<Delegation>) -> Self {
Self {
account: stake_account.account,
stake_state: stake_account.stake_state,
_phantom: PhantomData,
}
}
}
impl<T> From<StakeAccount<T>> for (AccountSharedData, StakeState) {
#[inline]
fn from(stake_account: StakeAccount<T>) -> Self {

View File

@ -124,11 +124,10 @@ impl StakesCache {
pub(crate) fn handle_invalid_keys(
&self,
invalid_stake_keys: DashMap<Pubkey, InvalidCacheEntryReason>,
invalid_vote_keys: DashMap<Pubkey, InvalidCacheEntryReason>,
current_slot: Slot,
) {
if invalid_stake_keys.is_empty() && invalid_vote_keys.is_empty() {
if invalid_vote_keys.is_empty() {
return;
}
@ -136,16 +135,6 @@ impl StakesCache {
// not properly evicted in normal operation.
let mut stakes = self.0.write().unwrap();
for (stake_pubkey, reason) in invalid_stake_keys {
stakes.remove_stake_delegation(&stake_pubkey);
datapoint_warn!(
"bank-stake_delegation_accounts-invalid-account",
("slot", current_slot as i64, i64),
("stake-address", format!("{stake_pubkey:?}"), String),
("reason", reason.to_i64().unwrap_or_default(), i64),
);
}
for (vote_pubkey, reason) in invalid_vote_keys {
stakes.remove_vote_account(&vote_pubkey);
datapoint_warn!(