From 95ac00d30a6789a11d46865a6c305fa0b9fad241 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 6 Oct 2021 11:20:50 +0000 Subject: [PATCH] Make rewards tracer async friendly (backport #20452) (#20456) * Make rewards tracer async friendly (#20452) (cherry picked from commit 250a8503fe620df9fa5f7ca5533283f80258c2b9) # Conflicts: # Cargo.lock # ledger-tool/Cargo.toml # runtime/src/bank.rs * fix conflicts Co-authored-by: Justin Starry --- Cargo.lock | 1 + ledger-tool/Cargo.toml | 1 + ledger-tool/src/main.rs | 48 +++++++++++++++---------- programs/stake/src/stake_state.rs | 60 +++++++++++++++---------------- runtime/src/bank.rs | 45 +++++++++-------------- sdk/bpf/syscalls.txt | 26 ++++++++++++++ 6 files changed, 105 insertions(+), 76 deletions(-) create mode 100644 sdk/bpf/syscalls.txt diff --git a/Cargo.lock b/Cargo.lock index 051a59859f..3e7794533e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4796,6 +4796,7 @@ dependencies = [ "bytecount", "clap", "csv", + "dashmap", "futures 0.3.8", "futures-util", "histogram", diff --git a/ledger-tool/Cargo.toml b/ledger-tool/Cargo.toml index f68f2f1c7b..52ed6cf336 100644 --- a/ledger-tool/Cargo.toml +++ b/ledger-tool/Cargo.toml @@ -14,6 +14,7 @@ bs58 = "0.3.1" bytecount = "0.6.0" clap = "2.33.1" csv = "1.1.3" +dashmap = "4.0.2" futures = "0.3.8" futures-util = "0.3.5" histogram = "*" diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 475ca842da..ad63139e67 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -3,6 +3,7 @@ use clap::{ crate_description, crate_name, value_t, value_t_or_exit, values_t_or_exit, App, AppSettings, Arg, ArgMatches, SubCommand, }; +use dashmap::DashMap; use itertools::Itertools; use log::*; use regex::Regex; @@ -57,7 +58,7 @@ use std::{ path::{Path, PathBuf}, process::{exit, Command, Stdio}, str::FromStr, - sync::Arc, + sync::{Arc, RwLock}, }; mod bigtable; @@ -2345,15 +2346,16 @@ fn main() { skipped_reasons: String, } use solana_stake_program::stake_state::InflationPointCalculationEvent; - let mut stake_calcuration_details: HashMap = - HashMap::new(); - let mut last_point_value = None; + let stake_calculation_details: DashMap = + DashMap::new(); + let last_point_value = Arc::new(RwLock::new(None)); let tracer = |event: &RewardCalculationEvent| { // Currently RewardCalculationEvent enum has only Staking variant // because only staking tracing is supported! #[allow(irrefutable_let_patterns)] if let RewardCalculationEvent::Staking(pubkey, event) = event { - let detail = stake_calcuration_details.entry(**pubkey).or_default(); + let mut detail = + stake_calculation_details.entry(**pubkey).or_default(); match event { InflationPointCalculationEvent::CalculatedPoints( epoch, @@ -2378,12 +2380,11 @@ fn main() { detail.point_value = Some(point_value.clone()); // we have duplicate copies of `PointValue`s for possible // miscalculation; do some minimum sanity check - let point_value = detail.point_value.clone(); - if point_value.is_some() { - if last_point_value.is_some() { - assert_eq!(last_point_value, point_value,); - } - last_point_value = point_value; + let mut last_point_value = last_point_value.write().unwrap(); + if let Some(last_point_value) = last_point_value.as_ref() { + assert_eq!(last_point_value, point_value); + } else { + *last_point_value = Some(point_value.clone()); } } InflationPointCalculationEvent::EffectiveStakeAtRewardedEpoch(stake) => { @@ -2488,16 +2489,17 @@ fn main() { }, ); - let mut unchanged_accounts = stake_calcuration_details - .keys() + let mut unchanged_accounts = stake_calculation_details + .iter() + .map(|entry| *entry.key()) .collect::>() .difference( &rewarded_accounts .iter() - .map(|(pubkey, ..)| *pubkey) + .map(|(pubkey, ..)| **pubkey) .collect(), ) - .map(|pubkey| (**pubkey, warped_bank.get_account(pubkey).unwrap())) + .map(|pubkey| (*pubkey, warped_bank.get_account(pubkey).unwrap())) .collect::>(); unchanged_accounts.sort_unstable_by_key(|(pubkey, account)| { (*account.owner(), account.lamports(), *pubkey) @@ -2518,7 +2520,9 @@ fn main() { if let Some(base_account) = base_bank.get_account(&pubkey) { let delta = warped_account.lamports() - base_account.lamports(); - let detail = stake_calcuration_details.get(&pubkey); + let detail_ref = stake_calculation_details.get(&pubkey); + let detail: Option<&CalculationDetail> = + detail_ref.as_ref().map(|detail_ref| detail_ref.value()); println!( "{:<45}({}): {} => {} (+{} {:>4.9}%) {:?}", format!("{}", pubkey), // format! is needed to pad/justify correctly. @@ -2641,10 +2645,18 @@ fn main() { ), commission: format_or_na(detail.map(|d| d.commission)), cluster_rewards: format_or_na( - last_point_value.as_ref().map(|pv| pv.rewards), + last_point_value + .read() + .unwrap() + .clone() + .map(|pv| pv.rewards), ), cluster_points: format_or_na( - last_point_value.as_ref().map(|pv| pv.points), + last_point_value + .read() + .unwrap() + .clone() + .map(|pv| pv.points), ), old_capitalization: base_bank.capitalization(), new_capitalization: warped_bank.capitalization(), diff --git a/programs/stake/src/stake_state.rs b/programs/stake/src/stake_state.rs index 0c68ed0756..06bf84de47 100644 --- a/programs/stake/src/stake_state.rs +++ b/programs/stake/src/stake_state.rs @@ -59,7 +59,7 @@ pub enum InflationPointCalculationEvent { Skipped(SkippedReason), } -pub(crate) fn null_tracer() -> Option { +pub(crate) fn null_tracer() -> Option { None:: } @@ -161,10 +161,10 @@ fn redeem_stake_rewards( point_value: &PointValue, vote_state: &VoteState, stake_history: Option<&StakeHistory>, - inflation_point_calc_tracer: &mut Option, + inflation_point_calc_tracer: Option, fix_activating_credits_observed: bool, ) -> Option<(u64, u64)> { - if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer { + if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { inflation_point_calc_tracer(&InflationPointCalculationEvent::CreditsObserved( stake.credits_observed, None, @@ -176,7 +176,7 @@ fn redeem_stake_rewards( point_value, vote_state, stake_history, - inflation_point_calc_tracer, + inflation_point_calc_tracer.as_ref(), fix_activating_credits_observed, ) .map(|(stakers_reward, voters_reward, credits_observed)| { @@ -196,7 +196,7 @@ fn calculate_stake_points( stake: &Stake, vote_state: &VoteState, stake_history: Option<&StakeHistory>, - inflation_point_calc_tracer: &mut Option, + inflation_point_calc_tracer: Option, ) -> u128 { calculate_stake_points_and_credits( stake, @@ -214,11 +214,11 @@ fn calculate_stake_points_and_credits( stake: &Stake, new_vote_state: &VoteState, stake_history: Option<&StakeHistory>, - inflation_point_calc_tracer: &mut Option, + inflation_point_calc_tracer: Option, ) -> (u128, u64) { // if there is no newer credits since observed, return no point if new_vote_state.credits() <= stake.credits_observed { - if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer { + if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { inflation_point_calc_tracer(&SkippedReason::ZeroCreditsAndReturnCurrent.into()); } return (0, stake.credits_observed); @@ -254,7 +254,7 @@ fn calculate_stake_points_and_credits( let earned_points = stake_amount * earned_credits; points += earned_points; - if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer { + if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { inflation_point_calc_tracer(&InflationPointCalculationEvent::CalculatedPoints( epoch, stake_amount, @@ -279,14 +279,14 @@ fn calculate_stake_rewards( point_value: &PointValue, vote_state: &VoteState, stake_history: Option<&StakeHistory>, - inflation_point_calc_tracer: &mut Option, + inflation_point_calc_tracer: Option, fix_activating_credits_observed: bool, ) -> Option<(u64, u64, u64)> { let (points, credits_observed) = calculate_stake_points_and_credits( stake, vote_state, stake_history, - inflation_point_calc_tracer, + inflation_point_calc_tracer.as_ref(), ); // Drive credits_observed forward unconditionally when rewards are disabled @@ -1105,11 +1105,11 @@ pub fn redeem_rewards( vote_state: &VoteState, point_value: &PointValue, stake_history: Option<&StakeHistory>, - inflation_point_calc_tracer: &mut Option, + inflation_point_calc_tracer: Option, fix_activating_credits_observed: bool, ) -> Result<(u64, u64), InstructionError> { if let StakeState::Stake(meta, mut stake) = stake_account.state()? { - if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer { + if let Some(inflation_point_calc_tracer) = inflation_point_calc_tracer.as_ref() { inflation_point_calc_tracer( &InflationPointCalculationEvent::EffectiveStakeAtRewardedEpoch( stake.stake(rewarded_epoch, stake_history), @@ -1160,7 +1160,7 @@ pub fn calculate_points( &stake, &vote_state, stake_history, - &mut null_tracer(), + null_tracer(), )) } else { Err(InstructionError::InvalidAccountData) @@ -3373,7 +3373,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3394,7 +3394,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3432,7 +3432,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3447,7 +3447,7 @@ mod tests { // no overflow on points assert_eq!( u128::from(stake.delegation.stake) * epoch_slots, - calculate_stake_points(&stake, &vote_state, None, &mut null_tracer()) + calculate_stake_points(&stake, &vote_state, None, null_tracer()) ); } @@ -3476,7 +3476,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3497,7 +3497,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3515,7 +3515,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3536,7 +3536,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3555,7 +3555,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3580,7 +3580,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3599,7 +3599,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3615,7 +3615,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3634,7 +3634,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3653,7 +3653,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3661,7 +3661,7 @@ 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, &mut null_tracer()) + calculate_stake_points_and_credits(&stake, &vote_state, None, null_tracer()) ); // get rewards and credits observed when not the activation epoch @@ -3683,7 +3683,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); @@ -3703,7 +3703,7 @@ mod tests { }, &vote_state, None, - &mut null_tracer(), + null_tracer(), true, ) ); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 6a14bb6f49..37f6cd0705 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -808,7 +808,7 @@ pub enum RewardCalculationEvent<'a, 'b> { Staking(&'a Pubkey, &'b InflationPointCalculationEvent), } -fn null_tracer() -> Option { +fn null_tracer() -> Option { None:: } @@ -1121,7 +1121,7 @@ impl Bank { /// Create a new bank that points to an immutable checkpoint of another bank. pub fn new_from_parent(parent: &Arc, collector_id: &Pubkey, slot: Slot) -> Self { - Self::_new_from_parent(parent, collector_id, slot, &mut null_tracer(), false) + Self::_new_from_parent(parent, collector_id, slot, null_tracer(), false) } pub fn new_from_parent_with_vote_only( @@ -1130,35 +1130,23 @@ impl Bank { slot: Slot, vote_only_bank: bool, ) -> Self { - Self::_new_from_parent( - parent, - collector_id, - slot, - &mut null_tracer(), - vote_only_bank, - ) + Self::_new_from_parent(parent, collector_id, slot, null_tracer(), vote_only_bank) } pub fn new_from_parent_with_tracer( parent: &Arc, collector_id: &Pubkey, slot: Slot, - reward_calc_tracer: impl FnMut(&RewardCalculationEvent), + reward_calc_tracer: impl Fn(&RewardCalculationEvent) + Send + Sync, ) -> Self { - Self::_new_from_parent( - parent, - collector_id, - slot, - &mut Some(reward_calc_tracer), - false, - ) + Self::_new_from_parent(parent, collector_id, slot, Some(reward_calc_tracer), false) } fn _new_from_parent( parent: &Arc, collector_id: &Pubkey, slot: Slot, - reward_calc_tracer: &mut Option, + reward_calc_tracer: Option, vote_only_bank: bool, ) -> Self { parent.freeze(); @@ -1840,7 +1828,7 @@ impl Bank { fn update_rewards( &mut self, prev_epoch: Epoch, - reward_calc_tracer: &mut Option, + reward_calc_tracer: Option, ) { if prev_epoch == self.epoch() { return; @@ -1944,7 +1932,7 @@ impl Bank { /// Filters out invalid pairs fn stake_delegation_accounts( &self, - reward_calc_tracer: &mut Option, + reward_calc_tracer: Option, ) -> HashMap, AccountSharedData)> { let mut accounts = HashMap::new(); @@ -1960,7 +1948,7 @@ impl Bank { ) { (Some(stake_account), Some(vote_account)) => { // call tracer to catch any illegal data if any - if let Some(reward_calc_tracer) = reward_calc_tracer { + if let Some(reward_calc_tracer) = reward_calc_tracer.as_ref() { reward_calc_tracer(&RewardCalculationEvent::Staking( stake_pubkey, &InflationPointCalculationEvent::Delegation( @@ -2005,12 +1993,13 @@ impl Bank { &mut self, rewarded_epoch: Epoch, rewards: u64, - reward_calc_tracer: &mut Option, + reward_calc_tracer: Option, fix_activating_credits_observed: bool, ) -> f64 { let stake_history = self.stakes.read().unwrap().history().clone(); - let mut stake_delegation_accounts = self.stake_delegation_accounts(reward_calc_tracer); + let mut stake_delegation_accounts = + self.stake_delegation_accounts(reward_calc_tracer.as_ref()); let points: u128 = stake_delegation_accounts .iter() @@ -2050,7 +2039,7 @@ impl Bank { for (stake_pubkey, stake_account) in stake_group.iter_mut() { // curry closure to add the contextual stake_pubkey - let mut reward_calc_tracer = reward_calc_tracer.as_mut().map(|outer| { + let reward_calc_tracer = reward_calc_tracer.as_ref().map(|outer| { let stake_pubkey = *stake_pubkey; // inner move |inner_event: &_| { @@ -2064,7 +2053,7 @@ impl Bank { &vote_state, &point_value, Some(&stake_history), - &mut reward_calc_tracer.as_mut(), + reward_calc_tracer, fix_activating_credits_observed, ); if let Ok((stakers_reward, _voters_reward)) = redeemed { @@ -7463,7 +7452,7 @@ pub(crate) mod tests { bank0.store_account_and_update_capitalization(&vote_id, &vote_account); let validator_points: u128 = bank0 - .stake_delegation_accounts(&mut null_tracer()) + .stake_delegation_accounts(null_tracer()) .iter() .flat_map(|(_vote_pubkey, (stake_group, vote_account))| { stake_group @@ -13246,7 +13235,7 @@ pub(crate) mod tests { vec![10_000; 2], ); let bank = Arc::new(Bank::new(&genesis_config)); - let stake_delegation_accounts = bank.stake_delegation_accounts(&mut null_tracer()); + let stake_delegation_accounts = bank.stake_delegation_accounts(null_tracer()); assert_eq!(stake_delegation_accounts.len(), 2); let mut vote_account = bank @@ -13285,7 +13274,7 @@ pub(crate) mod tests { ); // Accounts must be valid stake and vote accounts - let stake_delegation_accounts = bank.stake_delegation_accounts(&mut null_tracer()); + let stake_delegation_accounts = bank.stake_delegation_accounts(null_tracer()); assert_eq!(stake_delegation_accounts.len(), 0); } diff --git a/sdk/bpf/syscalls.txt b/sdk/bpf/syscalls.txt new file mode 100644 index 0000000000..ff9227eb04 --- /dev/null +++ b/sdk/bpf/syscalls.txt @@ -0,0 +1,26 @@ +abort +sol_panic_ +sol_log_ +sol_log_64_ +sol_log_compute_units_ +sol_log_pubkey +sol_create_program_address +sol_try_find_program_address +sol_sha256 +sol_keccak256 +sol_secp256k1_recover +sol_blake3 +sol_get_clock_sysvar +sol_get_epoch_schedule_sysvar +sol_get_fees_sysvar +sol_get_rent_sysvar +sol_memcpy_ +sol_memmove_ +sol_memcmp_ +sol_memset_ +sol_invoke_signed_c +sol_invoke_signed_rust +sol_alloc_free_ +sol_set_return_data +sol_get_return_data +sol_log_data