From 7171c95bdd5738b835150098cd3af3bcc3d0ada7 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Wed, 12 Jan 2022 11:19:11 +0800 Subject: [PATCH] Refactor: move sysvar cache to new module --- Cargo.lock | 2 ++ program-runtime/Cargo.toml | 2 ++ program-runtime/src/invoke_context.rs | 19 +++++------ program-runtime/src/lib.rs | 1 + program-runtime/src/sysvar_cache.rs | 39 +++++++++++++++++++++++ programs/bpf/Cargo.lock | 2 ++ programs/bpf_loader/src/syscalls.rs | 30 ++++++++---------- programs/stake/src/stake_instruction.rs | 21 +++++++------ programs/vote/src/vote_instruction.rs | 14 +++++---- runtime/src/bank.rs | 42 +++++-------------------- runtime/src/bank/sysvar_cache.rs | 33 +++++++++++++++++++ runtime/src/message_processor.rs | 26 ++++++++------- 12 files changed, 144 insertions(+), 87 deletions(-) create mode 100644 program-runtime/src/sysvar_cache.rs create mode 100644 runtime/src/bank/sysvar_cache.rs diff --git a/Cargo.lock b/Cargo.lock index ac6db7baac..d086b60180 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5514,6 +5514,8 @@ dependencies = [ "num-traits", "rustc_version 0.4.0", "serde", + "solana-frozen-abi 1.10.0", + "solana-frozen-abi-macro 1.10.0", "solana-logger 1.10.0", "solana-measure", "solana-sdk", diff --git a/program-runtime/Cargo.toml b/program-runtime/Cargo.toml index e0e5fe33f7..cb5a4cf334 100644 --- a/program-runtime/Cargo.toml +++ b/program-runtime/Cargo.toml @@ -19,6 +19,8 @@ log = "0.4.14" num-derive = { version = "0.3" } num-traits = { version = "0.2" } serde = { version = "1.0.129", features = ["derive", "rc"] } +solana-frozen-abi = { path = "../frozen-abi", version = "=1.10.0" } +solana-frozen-abi-macro = { path = "../frozen-abi/macro", version = "=1.10.0" } solana-measure = { path = "../measure", version = "=1.10.0" } solana-sdk = { path = "../sdk", version = "=1.10.0" } thiserror = "1.0" diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index d43f863efa..69754dcd6d 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -6,6 +6,7 @@ use { log_collector::LogCollector, native_loader::NativeLoader, pre_account::PreAccount, + sysvar_cache::SysvarCache, timings::{ExecuteDetailsTimings, ExecuteTimings}, }, solana_measure::measure::Measure, @@ -28,7 +29,7 @@ use { sysvar::Sysvar, transaction_context::{InstructionAccount, TransactionAccount, TransactionContext}, }, - std::{cell::RefCell, collections::HashMap, fmt::Debug, rc::Rc, sync::Arc}, + std::{borrow::Cow, cell::RefCell, collections::HashMap, fmt::Debug, rc::Rc, sync::Arc}, }; pub type ProcessInstructionWithContext = @@ -185,7 +186,7 @@ pub struct InvokeContext<'a> { rent: Rent, pre_accounts: Vec, builtin_programs: &'a [BuiltinProgram], - pub sysvars: &'a [(Pubkey, Vec)], + pub sysvar_cache: Cow<'a, SysvarCache>, log_collector: Option>>, compute_budget: ComputeBudget, current_compute_budget: ComputeBudget, @@ -205,7 +206,7 @@ impl<'a> InvokeContext<'a> { transaction_context: &'a mut TransactionContext, rent: Rent, builtin_programs: &'a [BuiltinProgram], - sysvars: &'a [(Pubkey, Vec)], + sysvar_cache: Cow<'a, SysvarCache>, log_collector: Option>>, compute_budget: ComputeBudget, executors: Rc>, @@ -221,7 +222,7 @@ impl<'a> InvokeContext<'a> { rent, pre_accounts: Vec::new(), builtin_programs, - sysvars, + sysvar_cache, log_collector, current_compute_budget: compute_budget, compute_budget, @@ -244,7 +245,7 @@ impl<'a> InvokeContext<'a> { transaction_context, Rent::default(), builtin_programs, - &[], + Cow::Owned(SysvarCache::default()), Some(LogCollector::new_ref()), ComputeBudget::default(), Rc::new(RefCell::new(Executors::default())), @@ -989,7 +990,7 @@ impl<'a> InvokeContext<'a> { /// Get the value of a sysvar by its id pub fn get_sysvar(&self, id: &Pubkey) -> Result { - self.sysvars + self.sysvar_cache .iter() .find_map(|(key, data)| { if id == key { @@ -1081,7 +1082,7 @@ pub fn mock_process_instruction_with_sysvars( transaction_accounts: Vec, instruction_accounts: Vec, expected_result: Result<(), InstructionError>, - sysvars: &[(Pubkey, Vec)], + sysvar_cache: &SysvarCache, process_instruction: ProcessInstructionWithContext, ) -> Vec { program_indices.insert(0, transaction_accounts.len()); @@ -1096,7 +1097,7 @@ pub fn mock_process_instruction_with_sysvars( ComputeBudget::default().max_invoke_depth.saturating_add(1), ); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); - invoke_context.sysvars = sysvars; + invoke_context.sysvar_cache = Cow::Borrowed(sysvar_cache); let result = invoke_context .push( &preparation.instruction_accounts, @@ -1127,7 +1128,7 @@ pub fn mock_process_instruction( transaction_accounts, instruction_accounts, expected_result, - &[], + &SysvarCache::default(), process_instruction, ) } diff --git a/program-runtime/src/lib.rs b/program-runtime/src/lib.rs index c72e6d94c6..e313be69c7 100644 --- a/program-runtime/src/lib.rs +++ b/program-runtime/src/lib.rs @@ -8,4 +8,5 @@ pub mod native_loader; pub mod neon_evm_program; pub mod pre_account; pub mod stable_log; +pub mod sysvar_cache; pub mod timings; diff --git a/program-runtime/src/sysvar_cache.rs b/program-runtime/src/sysvar_cache.rs new file mode 100644 index 0000000000..ba9629c992 --- /dev/null +++ b/program-runtime/src/sysvar_cache.rs @@ -0,0 +1,39 @@ +use { + solana_sdk::{ + account::{AccountSharedData, ReadableAccount}, + pubkey::Pubkey, + }, + std::ops::Deref, +}; + +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl ::solana_frozen_abi::abi_example::AbiExample for SysvarCache { + fn example() -> Self { + // SysvarCache is not Serialize so just rely on Default. + SysvarCache::default() + } +} + +#[derive(Default, Clone, Debug)] +pub struct SysvarCache(Vec<(Pubkey, Vec)>); + +impl Deref for SysvarCache { + type Target = Vec<(Pubkey, Vec)>; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl SysvarCache { + pub fn push_entry(&mut self, pubkey: Pubkey, data: Vec) { + self.0.push((pubkey, data)); + } + + pub fn update_entry(&mut self, pubkey: &Pubkey, new_account: &AccountSharedData) { + if let Some(position) = self.iter().position(|(id, _data)| id == pubkey) { + self.0[position].1 = new_account.data().to_vec(); + } else { + self.0.push((*pubkey, new_account.data().to_vec())); + } + } +} diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index e9aa2de5ab..13127c4052 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -3405,6 +3405,8 @@ dependencies = [ "num-traits", "rustc_version 0.4.0", "serde", + "solana-frozen-abi 1.10.0", + "solana-frozen-abi-macro 1.10.0", "solana-measure", "solana-sdk", "thiserror", diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index f2eaf36313..9ecaeb375f 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -2921,7 +2921,7 @@ impl<'a, 'b> SyscallObject for SyscallLogData<'a, 'b> { mod tests { use { super::*, - solana_program_runtime::invoke_context::InvokeContext, + solana_program_runtime::{invoke_context::InvokeContext, sysvar_cache::SysvarCache}, solana_rbpf::{ ebpf::HOST_ALIGN, memory_region::MemoryRegion, user_error::UserError, vm::Config, }, @@ -2929,7 +2929,7 @@ mod tests { account::AccountSharedData, bpf_loader, fee_calculator::FeeCalculator, hash::hashv, transaction_context::TransactionContext, }, - std::str::FromStr, + std::{borrow::Cow, str::FromStr}, }; macro_rules! assert_access_violation { @@ -3792,8 +3792,6 @@ mod tests { leader_schedule_epoch: 4, unix_timestamp: 5, }; - let mut data_clock = vec![]; - bincode::serialize_into(&mut data_clock, &src_clock).unwrap(); let src_epochschedule = EpochSchedule { slots_per_epoch: 1, leader_schedule_slot_offset: 2, @@ -3801,35 +3799,33 @@ mod tests { first_normal_epoch: 3, first_normal_slot: 4, }; - let mut data_epochschedule = vec![]; - bincode::serialize_into(&mut data_epochschedule, &src_epochschedule).unwrap(); let src_fees = Fees { fee_calculator: FeeCalculator { lamports_per_signature: 1, }, }; - let mut data_fees = vec![]; - bincode::serialize_into(&mut data_fees, &src_fees).unwrap(); let src_rent = Rent { lamports_per_byte_year: 1, exemption_threshold: 2.0, burn_percent: 3, }; - let mut data_rent = vec![]; - bincode::serialize_into(&mut data_rent, &src_rent).unwrap(); - let sysvars = [ - (sysvar::clock::id(), data_clock), - (sysvar::epoch_schedule::id(), data_epochschedule), - (sysvar::fees::id(), data_fees), - (sysvar::rent::id(), data_rent), - ]; + + let mut sysvar_cache = SysvarCache::default(); + sysvar_cache.push_entry(sysvar::clock::id(), bincode::serialize(&src_clock).unwrap()); + sysvar_cache.push_entry( + sysvar::epoch_schedule::id(), + bincode::serialize(&src_epochschedule).unwrap(), + ); + sysvar_cache.push_entry(sysvar::fees::id(), bincode::serialize(&src_fees).unwrap()); + sysvar_cache.push_entry(sysvar::rent::id(), bincode::serialize(&src_rent).unwrap()); + let program_id = Pubkey::new_unique(); let mut transaction_context = TransactionContext::new( vec![(program_id, AccountSharedData::new(0, 0, &bpf_loader::id()))], 1, ); let mut invoke_context = InvokeContext::new_mock(&mut transaction_context, &[]); - invoke_context.sysvars = &sysvars; + invoke_context.sysvar_cache = Cow::Owned(sysvar_cache); invoke_context.push(&[], &[0], &[]).unwrap(); // Test clock sysvar diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 63bfa229c6..81b943ce47 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -341,8 +341,9 @@ mod tests { super::*, crate::stake_state::{Meta, StakeState}, bincode::serialize, - solana_program_runtime::invoke_context::{ - mock_process_instruction, mock_process_instruction_with_sysvars, + solana_program_runtime::{ + invoke_context::{mock_process_instruction, mock_process_instruction_with_sysvars}, + sysvar_cache::SysvarCache, }, solana_sdk::{ account::{self, AccountSharedData}, @@ -354,7 +355,7 @@ mod tests { instruction::{self, LockupArgs}, state::{Authorized, Lockup, StakeAuthorize}, }, - sysvar::{stake_history::StakeHistory, Sysvar}, + sysvar::stake_history::StakeHistory, }, std::str::FromStr, }; @@ -436,8 +437,9 @@ mod tests { ) }) .collect(); - let mut data = Vec::with_capacity(sysvar::clock::Clock::size_of()); - bincode::serialize_into(&mut data, &sysvar::clock::Clock::default()).unwrap(); + let mut sysvar_cache = SysvarCache::default(); + let clock = Clock::default(); + sysvar_cache.push_entry(sysvar::clock::id(), bincode::serialize(&clock).unwrap()); mock_process_instruction_with_sysvars( &id(), Vec::new(), @@ -445,7 +447,7 @@ mod tests { transaction_accounts, instruction.accounts.clone(), expected_result, - &[(sysvar::clock::id(), data)], + &sysvar_cache, super::process_instruction, ) } @@ -1186,8 +1188,9 @@ mod tests { ) .unwrap(); - let mut data = Vec::with_capacity(sysvar::clock::Clock::size_of()); - bincode::serialize_into(&mut data, &sysvar::clock::Clock::default()).unwrap(); + let mut sysvar_cache = SysvarCache::default(); + let clock = Clock::default(); + sysvar_cache.push_entry(sysvar::clock::id(), bincode::serialize(&clock).unwrap()); mock_process_instruction_with_sysvars( &id(), Vec::new(), @@ -1215,7 +1218,7 @@ mod tests { }, ], Ok(()), - &[(sysvar::clock::id(), data)], + &sysvar_cache, super::process_instruction, ); } diff --git a/programs/vote/src/vote_instruction.rs b/programs/vote/src/vote_instruction.rs index 817d2a54e1..87c1535784 100644 --- a/programs/vote/src/vote_instruction.rs +++ b/programs/vote/src/vote_instruction.rs @@ -518,8 +518,9 @@ mod tests { use { super::*, bincode::serialize, - solana_program_runtime::invoke_context::{ - mock_process_instruction, mock_process_instruction_with_sysvars, + solana_program_runtime::{ + invoke_context::{mock_process_instruction, mock_process_instruction_with_sysvars}, + sysvar_cache::SysvarCache, }, solana_sdk::{ account::{self, Account, AccountSharedData}, @@ -579,12 +580,13 @@ mod tests { ) }) .collect(); + let mut sysvar_cache = SysvarCache::default(); let rent = Rent::default(); - let rent_sysvar = (sysvar::rent::id(), bincode::serialize(&rent).unwrap()); + sysvar_cache.push_entry(sysvar::rent::id(), bincode::serialize(&rent).unwrap()); let clock = Clock::default(); - let clock_sysvar = (sysvar::clock::id(), bincode::serialize(&clock).unwrap()); + sysvar_cache.push_entry(sysvar::clock::id(), bincode::serialize(&clock).unwrap()); let slot_hashes = SlotHashes::default(); - let slot_hashes_sysvar = ( + sysvar_cache.push_entry( sysvar::slot_hashes::id(), bincode::serialize(&slot_hashes).unwrap(), ); @@ -595,7 +597,7 @@ mod tests { transaction_accounts, instruction.accounts.clone(), expected_result, - &[rent_sysvar, clock_sysvar, slot_hashes_sysvar], + &sysvar_cache, super::process_instruction, ) } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index cb57a3cef2..131c9b4027 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -79,6 +79,7 @@ use { BuiltinProgram, Executor, Executors, ProcessInstructionWithContext, TransactionExecutor, }, log_collector::LogCollector, + sysvar_cache::SysvarCache, timings::ExecuteTimings, }, solana_sdk::{ @@ -160,6 +161,7 @@ use { }, }; +mod sysvar_cache; mod transaction_account_state_info; pub const SECONDS_PER_YEAR: f64 = 365.25 * 24.0 * 60.0 * 60.0; @@ -1186,7 +1188,7 @@ pub struct Bank { pub cost_tracker: RwLock, - sysvar_cache: RwLock)>>, + sysvar_cache: RwLock, /// Current size of the accounts data. Used when processing messages to enforce a limit on its /// maximum size. @@ -1331,7 +1333,7 @@ impl Bank { freeze_started: AtomicBool::default(), vote_only_bank: false, cost_tracker: RwLock::::default(), - sysvar_cache: RwLock::new(Vec::new()), + sysvar_cache: RwLock::::default(), accounts_data_len: AtomicU64::default(), }; @@ -1584,7 +1586,7 @@ impl Bank { )), freeze_started: AtomicBool::new(false), cost_tracker: RwLock::new(CostTracker::default()), - sysvar_cache: RwLock::new(Vec::new()), + sysvar_cache: RwLock::new(SysvarCache::default()), accounts_data_len: AtomicU64::new(parent.load_accounts_data_len()), }; @@ -1772,7 +1774,7 @@ impl Bank { freeze_started: AtomicBool::new(fields.hash != Hash::default()), vote_only_bank: false, cost_tracker: RwLock::new(CostTracker::default()), - sysvar_cache: RwLock::new(Vec::new()), + sysvar_cache: RwLock::new(SysvarCache::default()), accounts_data_len: AtomicU64::new(accounts_data_len), }; bank.finish_init( @@ -1955,11 +1957,7 @@ impl Bank { // Update the entry in the cache let mut sysvar_cache = self.sysvar_cache.write().unwrap(); - if let Some(position) = sysvar_cache.iter().position(|(id, _data)| id == pubkey) { - sysvar_cache[position].1 = new_account.data().to_vec(); - } else { - sysvar_cache.push((*pubkey, new_account.data().to_vec())); - } + sysvar_cache.update_entry(pubkey, &new_account); } fn inherit_specially_retained_account_fields( @@ -2109,17 +2107,6 @@ impl Bank { }); } - fn fill_sysvar_cache(&mut self) { - let mut sysvar_cache = self.sysvar_cache.write().unwrap(); - for id in sysvar::ALL_IDS.iter() { - if !sysvar_cache.iter().any(|(key, _data)| key == id) { - if let Some(account) = self.get_account_with_fixed_root(id) { - sysvar_cache.push((*id, account.data().to_vec())); - } - } - } - } - pub fn get_slot_history(&self) -> SlotHistory { from_account(&self.get_account(&sysvar::slot_history::id()).unwrap()).unwrap() } @@ -3629,21 +3616,6 @@ impl Bank { Arc::make_mut(&mut cache).remove(pubkey); } - /// Get the value of a cached sysvar by its id - pub fn get_cached_sysvar(&self, id: &Pubkey) -> Option { - self.sysvar_cache - .read() - .unwrap() - .iter() - .find_map(|(key, data)| { - if id == key { - bincode::deserialize(data).ok() - } else { - None - } - }) - } - pub fn load_lookup_table_addresses( &self, address_table_lookups: &[MessageAddressTableLookup], diff --git a/runtime/src/bank/sysvar_cache.rs b/runtime/src/bank/sysvar_cache.rs new file mode 100644 index 0000000000..c050276144 --- /dev/null +++ b/runtime/src/bank/sysvar_cache.rs @@ -0,0 +1,33 @@ +use { + super::Bank, + solana_sdk::{ + account::ReadableAccount, + pubkey::Pubkey, + sysvar::{self, Sysvar}, + }, +}; + +impl Bank { + pub(crate) fn fill_sysvar_cache(&mut self) { + let mut sysvar_cache = self.sysvar_cache.write().unwrap(); + for id in sysvar::ALL_IDS.iter() { + if !sysvar_cache.iter().any(|(key, _data)| key == id) { + if let Some(account) = self.get_account_with_fixed_root(id) { + sysvar_cache.push_entry(*id, account.data().to_vec()); + } + } + } + } + + /// Get the value of a cached sysvar by its id + pub fn get_cached_sysvar(&self, id: &Pubkey) -> Option { + let sysvar_cache = self.sysvar_cache.read().unwrap(); + sysvar_cache.iter().find_map(|(key, data)| { + if id == key { + bincode::deserialize(data).ok() + } else { + None + } + }) + } +} diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 758ecba692..c49bbb74c7 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -5,6 +5,7 @@ use { instruction_recorder::InstructionRecorder, invoke_context::{BuiltinProgram, Executors, InvokeContext}, log_collector::LogCollector, + sysvar_cache::SysvarCache, timings::ExecuteTimings, }, solana_sdk::{ @@ -14,14 +15,13 @@ use { hash::Hash, message::SanitizedMessage, precompiles::is_precompile, - pubkey::Pubkey, rent::Rent, saturating_add_assign, sysvar::instructions, transaction::TransactionError, transaction_context::{InstructionAccount, TransactionContext}, }, - std::{cell::RefCell, rc::Rc, sync::Arc}, + std::{borrow::Cow, cell::RefCell, rc::Rc, sync::Arc}, }; #[derive(Debug, Default, Clone, Deserialize, Serialize)] @@ -62,7 +62,7 @@ impl MessageProcessor { feature_set: Arc, compute_budget: ComputeBudget, timings: &mut ExecuteTimings, - sysvars: &[(Pubkey, Vec)], + sysvar_cache: &SysvarCache, blockhash: Hash, lamports_per_signature: u64, current_accounts_data_len: u64, @@ -71,7 +71,7 @@ impl MessageProcessor { transaction_context, rent, builtin_programs, - sysvars, + Cow::Borrowed(sysvar_cache), log_collector, compute_budget, executors, @@ -166,6 +166,7 @@ mod tests { instruction::{AccountMeta, Instruction, InstructionError}, message::Message, native_loader::{self, create_loadable_account_for_test}, + pubkey::Pubkey, secp256k1_instruction::new_secp256k1_instruction, secp256k1_program, }, @@ -257,6 +258,7 @@ mod tests { )], Some(transaction_context.get_key_of_account_at_index(0)), )); + let sysvar_cache = SysvarCache::default(); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -269,7 +271,7 @@ mod tests { Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), &mut ExecuteTimings::default(), - &[], + &sysvar_cache, Hash::default(), 0, 0, @@ -310,7 +312,7 @@ mod tests { Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), &mut ExecuteTimings::default(), - &[], + &sysvar_cache, Hash::default(), 0, 0, @@ -343,7 +345,7 @@ mod tests { Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), &mut ExecuteTimings::default(), - &[], + &sysvar_cache, Hash::default(), 0, 0, @@ -457,6 +459,7 @@ mod tests { )], Some(transaction_context.get_key_of_account_at_index(0)), )); + let sysvar_cache = SysvarCache::default(); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -469,7 +472,7 @@ mod tests { Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), &mut ExecuteTimings::default(), - &[], + &sysvar_cache, Hash::default(), 0, 0, @@ -503,7 +506,7 @@ mod tests { Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), &mut ExecuteTimings::default(), - &[], + &sysvar_cache, Hash::default(), 0, 0, @@ -534,7 +537,7 @@ mod tests { Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), &mut ExecuteTimings::default(), - &[], + &sysvar_cache, Hash::default(), 0, 0, @@ -595,6 +598,7 @@ mod tests { ], None, )); + let sysvar_cache = SysvarCache::default(); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -607,7 +611,7 @@ mod tests { Arc::new(FeatureSet::all_enabled()), ComputeBudget::new(), &mut ExecuteTimings::default(), - &[], + &sysvar_cache, Hash::default(), 0, 0,