Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Add more reporting for invalid stake cache members and prune them (backport #21654) #21741

Merged
mergify merged 1 commit into from
Dec 10, 2021
Merged

Add more reporting for invalid stake cache members and prune them (backport #21654) #21741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions programs/bpf/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions runtime/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ lazy_static = "1.4.0"
log = "0.4.14"
memmap2 = "0.5.0"
num_cpus = "1.13.0"
num-derive = { version = "0.3" }
num-traits = { version = "0.2" }
ouroboros = "0.13.0"
rand = "0.7.0"
rayon = "1.5.1"
Expand Down
223 changes: 150 additions & 73 deletions runtime/src/bank.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ use {
dashmap::DashMap,
itertools::Itertools,
log::*,
num_derive::ToPrimitive,
num_traits::ToPrimitive,
rayon::{
iter::{IntoParallelIterator, IntoParallelRefIterator, ParallelIterator},
ThreadPool, ThreadPoolBuilder,
Expand Down Expand Up @@ -1054,6 +1056,19 @@ struct VoteWithStakeDelegations {
delegations: Vec<(Pubkey, (StakeState, AccountSharedData))>,
}

#[derive(Debug, Clone, PartialEq, ToPrimitive)]
enum InvalidReason {
Missing,
BadState,
WrongOwner,
}

struct LoadVoteAndStakeAccountsResult {
vote_with_stake_delegations_map: DashMap<Pubkey, VoteWithStakeDelegations>,
invalid_stake_keys: DashMap<Pubkey, InvalidReason>,
invalid_vote_keys: DashMap<Pubkey, InvalidReason>,
}

#[derive(Debug, Default)]
pub struct NewBankOptions {
pub vote_only_bank: bool,
Expand Down Expand Up @@ -2166,97 +2181,147 @@ impl Bank {
&self,
thread_pool: &ThreadPool,
reward_calc_tracer: Option<impl Fn(&RewardCalculationEvent) + Send + Sync>,
) -> DashMap<Pubkey, VoteWithStakeDelegations> {
) -> LoadVoteAndStakeAccountsResult {
let stakes = self.stakes.read().unwrap();
let accounts = DashMap::with_capacity(stakes.vote_accounts().as_ref().len());
let vote_with_stake_delegations_map =
DashMap::with_capacity(stakes.vote_accounts().as_ref().len());
let invalid_stake_keys: DashMap<Pubkey, InvalidReason> = DashMap::new();
let invalid_vote_keys: DashMap<Pubkey, InvalidReason> = DashMap::new();

thread_pool.install(|| {
stakes
.stake_delegations()
.par_iter()
.for_each(|(stake_pubkey, delegation)| {
let vote_pubkey = &delegation.voter_pubkey;
let stake_account = match self.get_account_with_fixed_root(stake_pubkey) {
Some(stake_account) => stake_account,
None => return,
};
if invalid_vote_keys.contains_key(vote_pubkey) {
return;
}

// fetch vote account from stakes cache if it hasn't been cached locally
let fetched_vote_account = if !accounts.contains_key(vote_pubkey) {
let vote_account = match self.get_account_with_fixed_root(vote_pubkey) {
Some(vote_account) => vote_account,
None => return,
};
let stake_delegation = match self.get_account_with_fixed_root(stake_pubkey) {
Some(stake_account) => {
if stake_account.owner() != &solana_stake_program::id() {
invalid_stake_keys.insert(*stake_pubkey, InvalidReason::WrongOwner);
return;
}

let vote_state: VoteState =
match StateMut::<VoteStateVersions>::state(&vote_account) {
Ok(vote_state) => vote_state.convert_to_current(),
Err(err) => {
debug!(
"failed to deserialize vote account {}: {}",
vote_pubkey, err
);
match stake_account.state().ok() {
Some(stake_state) => (*stake_pubkey, (stake_state, stake_account)),
None => {
invalid_stake_keys
.insert(*stake_pubkey, InvalidReason::BadState);
return;
}
};
}
}
None => {
invalid_stake_keys.insert(*stake_pubkey, InvalidReason::Missing);
return;
}
};

Some((vote_state, vote_account))
let mut vote_delegations = if let Some(vote_delegations) =
vote_with_stake_delegations_map.get_mut(vote_pubkey)
{
vote_delegations
} else {
None
};
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, InvalidReason::WrongOwner);
return;
}
vote_account
}
None => {
invalid_vote_keys.insert(*vote_pubkey, InvalidReason::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, InvalidReason::BadState);
return;
};

let fetched_vote_account_owner = fetched_vote_account
.as_ref()
.map(|(_vote_state, vote_account)| vote_account.owner());
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,
fetched_vote_account_owner
.cloned()
.unwrap_or_else(solana_vote_program::id),
solana_vote_program::id(),
),
));
}

// filter invalid delegation accounts
if stake_account.owner() != &solana_stake_program::id()
|| (fetched_vote_account_owner.is_some()
&& fetched_vote_account_owner != Some(&solana_vote_program::id()))
{
datapoint_warn!(
"bank-stake_delegation_accounts-invalid-account",
("slot", self.slot() as i64, i64),
("stake-address", format!("{:?}", stake_pubkey), String),
("vote-address", format!("{:?}", vote_pubkey), String),
);
return;
}
vote_delegations.delegations.push(stake_delegation);
});
});

let stake_delegation = match stake_account.state().ok() {
Some(stake_state) => (*stake_pubkey, (stake_state, stake_account)),
None => return,
};
LoadVoteAndStakeAccountsResult {
vote_with_stake_delegations_map,
invalid_vote_keys,
invalid_stake_keys,
}
}

if let Some((vote_state, vote_account)) = fetched_vote_account {
accounts
.entry(*vote_pubkey)
.or_insert_with(|| VoteWithStakeDelegations {
vote_state: Arc::new(vote_state),
vote_account,
delegations: vec![],
});
}
fn handle_invalid_stakes_cache_keys(
&self,
invalid_stake_keys: DashMap<Pubkey, InvalidReason>,
invalid_vote_keys: DashMap<Pubkey, InvalidReason>,
) {
if invalid_stake_keys.is_empty() && invalid_vote_keys.is_empty() {
return;
}

if let Some(mut stake_delegation_accounts) = accounts.get_mut(vote_pubkey) {
stake_delegation_accounts.delegations.push(stake_delegation);
}
});
});
// Prune invalid stake delegations and vote accounts that were
// not properly evicted in normal operation.
let mut maybe_stakes_cache = if self
.feature_set
.is_active(&feature_set::evict_invalid_stakes_cache_entries::id())
{
Some(self.stakes.write().unwrap())
} else {
None
};

for (stake_pubkey, reason) in invalid_stake_keys {
if let Some(stakes_cache) = maybe_stakes_cache.as_mut() {
stakes_cache.remove_stake_delegation(&stake_pubkey);
}
datapoint_warn!(
"bank-stake_delegation_accounts-invalid-account",
("slot", self.slot() as i64, i64),
("stake-address", format!("{:?}", stake_pubkey), String),
("reason", reason.to_i64().unwrap_or_default(), i64),
);
}

accounts
for (vote_pubkey, reason) in invalid_vote_keys {
if let Some(stakes_cache) = maybe_stakes_cache.as_mut() {
stakes_cache.remove_vote_account(&vote_pubkey);
}
datapoint_warn!(
"bank-stake_delegation_accounts-invalid-account",
("slot", self.slot() as i64, i64),
("vote-address", format!("{:?}", vote_pubkey), String),
("reason", reason.to_i64().unwrap_or_default(), i64),
);
}
}

/// iterate over all stakes, redeem vote credits for each stake we can
Expand All @@ -2270,13 +2335,22 @@ impl Bank {
thread_pool: &ThreadPool,
) -> f64 {
let stake_history = self.stakes.read().unwrap().history().clone();
let vote_and_stake_accounts = self.load_vote_and_stake_accounts_with_thread_pool(
thread_pool,
reward_calc_tracer.as_ref(),
);
let vote_with_stake_delegations_map = {
let LoadVoteAndStakeAccountsResult {
vote_with_stake_delegations_map,
invalid_stake_keys,
invalid_vote_keys,
} = self.load_vote_and_stake_accounts_with_thread_pool(
thread_pool,
reward_calc_tracer.as_ref(),
);

self.handle_invalid_stakes_cache_keys(invalid_stake_keys, invalid_vote_keys);
vote_with_stake_delegations_map
};

let points: u128 = thread_pool.install(|| {
vote_and_stake_accounts
vote_with_stake_delegations_map
.par_iter()
.map(|entry| {
let VoteWithStakeDelegations {
Expand Down Expand Up @@ -2307,8 +2381,8 @@ impl Bank {
// pay according to point value
let point_value = PointValue { rewards, points };
let vote_account_rewards: DashMap<Pubkey, (AccountSharedData, u8, u64, bool)> =
DashMap::with_capacity(vote_and_stake_accounts.len());
let stake_delegation_iterator = vote_and_stake_accounts.into_par_iter().flat_map(
DashMap::with_capacity(vote_with_stake_delegations_map.len());
let stake_delegation_iterator = vote_with_stake_delegations_map.into_par_iter().flat_map(
|(
vote_pubkey,
VoteWithStakeDelegations {
Expand Down Expand Up @@ -8279,6 +8353,7 @@ pub(crate) mod tests {
let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap();
let validator_points: u128 = bank0
.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
.vote_with_stake_delegations_map
.into_iter()
.map(
|(
Expand Down Expand Up @@ -14284,8 +14359,9 @@ pub(crate) mod tests {
);
let bank = Arc::new(Bank::new_for_tests(&genesis_config));
let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap();
let vote_and_stake_accounts =
bank.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer());
let vote_and_stake_accounts = bank
.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
.vote_with_stake_delegations_map;
assert_eq!(vote_and_stake_accounts.len(), 2);

let mut vote_account = bank
Expand Down Expand Up @@ -14325,8 +14401,9 @@ pub(crate) mod tests {

// Accounts must be valid stake and vote accounts
let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap();
let vote_and_stake_accounts =
bank.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer());
let vote_and_stake_accounts = bank
.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer())
.vote_with_stake_delegations_map;
assert_eq!(vote_and_stake_accounts.len(), 0);
}

Expand Down
12 changes: 12 additions & 0 deletions runtime/src/stakes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,18 @@ impl Stakes {
&& account.data().len() >= std::mem::size_of::<StakeState>()
}

pub fn remove_vote_account(&mut self, vote_pubkey: &Pubkey) {
self.vote_accounts.remove(vote_pubkey);
}

pub fn remove_stake_delegation(&mut self, stake_pubkey: &Pubkey) {
if let Some(removed_delegation) = self.stake_delegations.remove(stake_pubkey) {
let removed_stake = removed_delegation.stake(self.epoch, Some(&self.stake_history));
self.vote_accounts
.sub_stake(&removed_delegation.voter_pubkey, removed_stake);
}
}

pub fn store(
&mut self,
pubkey: &Pubkey,
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ pub mod reject_non_rent_exempt_vote_withdraws {
solana_sdk::declare_id!("7txXZZD6Um59YoLMF7XUNimbMjsqsWhc7g2EniiTrmp1");
}

pub mod evict_invalid_stakes_cache_entries {
solana_sdk::declare_id!("EMX9Q7TVFAmQ9V1CggAkhMzhXSg8ECp7fHrWQX2G1chf");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -328,6 +332,7 @@ lazy_static! {
(leave_nonce_on_success::id(), "leave nonce as is on success"),
(reject_empty_instruction_without_program::id(), "fail instructions which have native_loader as program_id directly"),
(reject_non_rent_exempt_vote_withdraws::id(), "fail vote withdraw instructions which leave the account non-rent-exempt"),
(evict_invalid_stakes_cache_entries::id(), "evict invalid stakes cache entries on epoch boundaries"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down