diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 2fd8524d7454b8..377741e415e8ef 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -606,6 +606,7 @@ impl Consumer { &mut execute_and_commit_timings.execute_timings, TransactionProcessingConfig { account_overrides: None, + check_program_modification_slot: bank.check_program_modification_slot(), compute_budget: bank.compute_budget(), log_messages_bytes_limit: self.log_messages_bytes_limit, limit_to_load_programs: true, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 574bc0ee5e1827..e14460f8eed901 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -98,10 +98,7 @@ use { solana_perf::perf_libs, solana_program_runtime::{ invoke_context::BuiltinFunctionWithContext, - loaded_programs::{ - ProgramCacheEntry, ProgramCacheEntryOwner, ProgramCacheEntryType, - ProgramCacheMatchCriteria, - }, + loaded_programs::{ProgramCacheEntry, ProgramCacheEntryOwner, ProgramCacheEntryType}, timings::{ExecuteTimingType, ExecuteTimings}, }, solana_sdk::{ @@ -109,8 +106,7 @@ use { create_account_shared_data_with_fields as create_account, from_account, Account, AccountSharedData, InheritableAccountFields, ReadableAccount, WritableAccount, }, - account_utils::StateMut, - bpf_loader_upgradeable::{self, UpgradeableLoaderState}, + bpf_loader_upgradeable, clock::{ BankId, Epoch, Slot, SlotCount, SlotIndex, UnixTimestamp, DEFAULT_HASHES_PER_TICK, DEFAULT_TICKS_PER_SECOND, INITIAL_RENT_EPOCH, MAX_PROCESSING_AGE, @@ -133,7 +129,6 @@ use { incinerator, inflation::Inflation, inner_instruction::InnerInstructions, - loader_v4, message::{AccountKeys, SanitizedMessage}, native_loader, native_token::LAMPORTS_PER_SOL, @@ -3407,6 +3402,7 @@ impl Bank { &mut timings, TransactionProcessingConfig { account_overrides: Some(&account_overrides), + check_program_modification_slot: self.check_program_modification_slot, compute_budget: self.compute_budget(), log_messages_bytes_limit: None, limit_to_load_programs: true, @@ -4840,6 +4836,7 @@ impl Bank { timings, TransactionProcessingConfig { account_overrides: None, + check_program_modification_slot: self.check_program_modification_slot, compute_budget: self.compute_budget(), log_messages_bytes_limit, limit_to_load_programs: false, @@ -6751,8 +6748,12 @@ impl Bank { false } - pub fn check_program_modification_slot(&mut self) { - self.check_program_modification_slot = true; + pub fn check_program_modification_slot(&self) -> bool { + self.check_program_modification_slot + } + + pub fn set_check_program_modification_slot(&mut self, check: bool) { + self.check_program_modification_slot = check; } pub fn fee_structure(&self) -> &FeeStructure { @@ -6767,40 +6768,6 @@ impl Bank { self.transaction_processor .add_builtin(self, program_id, name, builtin) } - - /// Find the slot in which the program was most recently modified. - /// Returns slot 0 for programs deployed with v1/v2 loaders, since programs deployed - /// with those loaders do not retain deployment slot information. - /// Returns an error if the program's account state can not be found or parsed. - fn program_modification_slot(&self, pubkey: &Pubkey) -> transaction::Result { - let program = self - .get_account(pubkey) - .ok_or(TransactionError::ProgramAccountNotFound)?; - if bpf_loader_upgradeable::check_id(program.owner()) { - if let Ok(UpgradeableLoaderState::Program { - programdata_address, - }) = program.state() - { - let programdata = self - .get_account(&programdata_address) - .ok_or(TransactionError::ProgramAccountNotFound)?; - if let Ok(UpgradeableLoaderState::ProgramData { - slot, - upgrade_authority_address: _, - }) = programdata.state() - { - return Ok(slot); - } - } - Err(TransactionError::ProgramAccountNotFound) - } else if loader_v4::check_id(program.owner()) { - let state = solana_loader_v4_program::get_state(program.data()) - .map_err(|_| TransactionError::ProgramAccountNotFound)?; - Ok(state.slot) - } else { - Ok(0) - } - } } impl TransactionProcessingCallback for Bank { @@ -6820,17 +6787,6 @@ impl TransactionProcessingCallback for Bank { .map(|(acc, _)| acc) } - fn get_program_match_criteria(&self, program: &Pubkey) -> ProgramCacheMatchCriteria { - if self.check_program_modification_slot { - self.program_modification_slot(program) - .map_or(ProgramCacheMatchCriteria::Tombstone, |slot| { - ProgramCacheMatchCriteria::DeployedOnOrAfterSlot(slot) - }) - } else { - ProgramCacheMatchCriteria::NoCriteria - } - } - // NOTE: must hold idempotent for the same set of arguments /// Add a builtin program account fn add_builtin_account(&self, name: &str, program_id: &Pubkey) { diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index ba9e4d323535c5..264485123da3ab 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -73,7 +73,6 @@ use { incinerator, instruction::{AccountMeta, CompiledInstruction, Instruction, InstructionError}, loader_upgradeable_instruction::UpgradeableLoaderInstruction, - loader_v4::{LoaderV4State, LoaderV4Status}, message::{Message, MessageHeader, SanitizedMessage}, native_loader, native_token::{sol_to_lamports, LAMPORTS_PER_SOL}, @@ -13057,91 +13056,6 @@ fn test_deploy_last_epoch_slot() { assert_eq!(result_with_feature_enabled, Ok(())); } -#[test] -fn test_program_modification_slot_account_not_found() { - let genesis_config = GenesisConfig::default(); - let bank = Bank::new_for_tests(&genesis_config); - let key = Pubkey::new_unique(); - - let result = bank.program_modification_slot(&key); - assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); - - let mut account_data = AccountSharedData::new(100, 100, &bpf_loader_upgradeable::id()); - bank.store_account(&key, &account_data); - - let result = bank.program_modification_slot(&key); - assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); - - let state = UpgradeableLoaderState::Program { - programdata_address: Pubkey::new_unique(), - }; - account_data.set_data(bincode::serialize(&state).unwrap()); - bank.store_account(&key, &account_data); - - let result = bank.program_modification_slot(&key); - assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); - - account_data.set_owner(loader_v4::id()); - bank.store_account(&key, &account_data); - - let result = bank.program_modification_slot(&key); - assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); -} - -#[test] -fn test_program_modification_slot_success() { - let genesis_config = GenesisConfig::default(); - let bank = Bank::new_for_tests(&genesis_config); - - let key1 = Pubkey::new_unique(); - let key2 = Pubkey::new_unique(); - - let account_data = AccountSharedData::new_data( - 100, - &UpgradeableLoaderState::Program { - programdata_address: key2, - }, - &bpf_loader_upgradeable::id(), - ) - .unwrap(); - bank.store_account(&key1, &account_data); - - let account_data = AccountSharedData::new_data( - 100, - &UpgradeableLoaderState::ProgramData { - slot: 77, - upgrade_authority_address: None, - }, - &bpf_loader_upgradeable::id(), - ) - .unwrap(); - bank.store_account(&key2, &account_data); - - let result = bank.program_modification_slot(&key1); - assert_eq!(result.unwrap(), 77); - - let state = LoaderV4State { - slot: 58, - authority_address: Pubkey::new_unique(), - status: LoaderV4Status::Deployed, - }; - let encoded = unsafe { - std::mem::transmute::<&LoaderV4State, &[u8; LoaderV4State::program_data_offset()]>(&state) - }; - let mut account_data = AccountSharedData::new(100, encoded.len(), &loader_v4::id()); - account_data.set_data(encoded.to_vec()); - bank.store_account(&key1, &account_data); - - let result = bank.program_modification_slot(&key1); - assert_eq!(result.unwrap(), 58); - - account_data.set_owner(Pubkey::new_unique()); - bank.store_account(&key2, &account_data); - - let result = bank.program_modification_slot(&key2); - assert_eq!(result.unwrap(), 0); -} - #[test] fn test_blockhash_last_valid_block_height() { let genesis_config = GenesisConfig::default(); diff --git a/runtime/src/bank_forks.rs b/runtime/src/bank_forks.rs index 264974fca536cb..bbdd2d270a189d 100644 --- a/runtime/src/bank_forks.rs +++ b/runtime/src/bank_forks.rs @@ -225,7 +225,7 @@ impl BankForks { pub fn insert(&mut self, mut bank: Bank) -> BankWithScheduler { if self.root.load(Ordering::Relaxed) < self.highest_slot_at_startup { - bank.check_program_modification_slot(); + bank.set_check_program_modification_slot(true); } let bank = Arc::new(bank); diff --git a/svm/src/program_loader.rs b/svm/src/program_loader.rs index ce790b004173af..f77780fc9fecfa 100644 --- a/svm/src/program_loader.rs +++ b/svm/src/program_loader.rs @@ -11,11 +11,12 @@ use { account::{AccountSharedData, ReadableAccount}, account_utils::StateMut, bpf_loader, bpf_loader_deprecated, - bpf_loader_upgradeable::UpgradeableLoaderState, + bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::Slot, instruction::InstructionError, loader_v4::{self, LoaderV4State, LoaderV4Status}, pubkey::Pubkey, + transaction::{self, TransactionError}, }, solana_type_overrides::sync::Arc, }; @@ -217,6 +218,43 @@ pub fn load_program_with_pubkey( Some(Arc::new(loaded_program)) } +/// Find the slot in which the program was most recently modified. +/// Returns slot 0 for programs deployed with v1/v2 loaders, since programs deployed +/// with those loaders do not retain deployment slot information. +/// Returns an error if the program's account state can not be found or parsed. +pub(crate) fn get_program_modification_slot( + callbacks: &CB, + pubkey: &Pubkey, +) -> transaction::Result { + let program = callbacks + .get_account_shared_data(pubkey) + .ok_or(TransactionError::ProgramAccountNotFound)?; + if bpf_loader_upgradeable::check_id(program.owner()) { + if let Ok(UpgradeableLoaderState::Program { + programdata_address, + }) = program.state() + { + let programdata = callbacks + .get_account_shared_data(&programdata_address) + .ok_or(TransactionError::ProgramAccountNotFound)?; + if let Ok(UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: _, + }) = programdata.state() + { + return Ok(slot); + } + } + Err(TransactionError::ProgramAccountNotFound) + } else if loader_v4::check_id(program.owner()) { + let state = solana_loader_v4_program::get_state(program.data()) + .map_err(|_| TransactionError::ProgramAccountNotFound)?; + Ok(state.slot) + } else { + Ok(0) + } +} + #[cfg(test)] mod tests { use { @@ -816,4 +854,111 @@ mod tests { ); } } + + #[test] + fn test_program_modification_slot_account_not_found() { + let mock_bank = MockBankCallback::default(); + + let key = Pubkey::new_unique(); + + let result = get_program_modification_slot(&mock_bank, &key); + assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); + + let mut account_data = AccountSharedData::new(100, 100, &bpf_loader_upgradeable::id()); + mock_bank + .account_shared_data + .borrow_mut() + .insert(key, account_data.clone()); + + let result = get_program_modification_slot(&mock_bank, &key); + assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); + + let state = UpgradeableLoaderState::Program { + programdata_address: Pubkey::new_unique(), + }; + account_data.set_data(bincode::serialize(&state).unwrap()); + mock_bank + .account_shared_data + .borrow_mut() + .insert(key, account_data.clone()); + + let result = get_program_modification_slot(&mock_bank, &key); + assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); + + account_data.set_owner(loader_v4::id()); + mock_bank + .account_shared_data + .borrow_mut() + .insert(key, account_data); + + let result = get_program_modification_slot(&mock_bank, &key); + assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound)); + } + + #[test] + fn test_program_modification_slot_success() { + let mock_bank = MockBankCallback::default(); + + let key1 = Pubkey::new_unique(); + let key2 = Pubkey::new_unique(); + + let account_data = AccountSharedData::new_data( + 100, + &UpgradeableLoaderState::Program { + programdata_address: key2, + }, + &bpf_loader_upgradeable::id(), + ) + .unwrap(); + mock_bank + .account_shared_data + .borrow_mut() + .insert(key1, account_data); + + let account_data = AccountSharedData::new_data( + 100, + &UpgradeableLoaderState::ProgramData { + slot: 77, + upgrade_authority_address: None, + }, + &bpf_loader_upgradeable::id(), + ) + .unwrap(); + mock_bank + .account_shared_data + .borrow_mut() + .insert(key2, account_data); + + let result = get_program_modification_slot(&mock_bank, &key1); + assert_eq!(result.unwrap(), 77); + + let state = LoaderV4State { + slot: 58, + authority_address: Pubkey::new_unique(), + status: LoaderV4Status::Deployed, + }; + let encoded = unsafe { + std::mem::transmute::<&LoaderV4State, &[u8; LoaderV4State::program_data_offset()]>( + &state, + ) + }; + let mut account_data = AccountSharedData::new(100, encoded.len(), &loader_v4::id()); + account_data.set_data(encoded.to_vec()); + mock_bank + .account_shared_data + .borrow_mut() + .insert(key1, account_data.clone()); + + let result = get_program_modification_slot(&mock_bank, &key1); + assert_eq!(result.unwrap(), 58); + + account_data.set_owner(Pubkey::new_unique()); + mock_bank + .account_shared_data + .borrow_mut() + .insert(key2, account_data); + + let result = get_program_modification_slot(&mock_bank, &key2); + assert_eq!(result.unwrap(), 0); + } } diff --git a/svm/src/transaction_processing_callback.rs b/svm/src/transaction_processing_callback.rs index bca549b12013cc..760a6606568798 100644 --- a/svm/src/transaction_processing_callback.rs +++ b/svm/src/transaction_processing_callback.rs @@ -1,7 +1,4 @@ -use { - solana_program_runtime::loaded_programs::ProgramCacheMatchCriteria, - solana_sdk::{account::AccountSharedData, pubkey::Pubkey}, -}; +use solana_sdk::{account::AccountSharedData, pubkey::Pubkey}; /// Runtime callbacks for transaction processing. pub trait TransactionProcessingCallback { @@ -9,9 +6,5 @@ pub trait TransactionProcessingCallback { fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option; - fn get_program_match_criteria(&self, _program: &Pubkey) -> ProgramCacheMatchCriteria { - ProgramCacheMatchCriteria::NoCriteria - } - fn add_builtin_account(&self, _name: &str, _program_id: &Pubkey) {} } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index fcbc2bef72dba2..2037489c0477f0 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -9,7 +9,7 @@ use { }, account_overrides::AccountOverrides, message_processor::MessageProcessor, - program_loader::load_program_with_pubkey, + program_loader::{get_program_modification_slot, load_program_with_pubkey}, rollback_accounts::RollbackAccounts, transaction_account_state_info::TransactionAccountStateInfo, transaction_error_metrics::TransactionErrorMetrics, @@ -104,6 +104,9 @@ pub struct TransactionProcessingConfig<'a> { /// Encapsulates overridden accounts, typically used for transaction /// simulation. pub account_overrides: Option<&'a AccountOverrides>, + /// Whether or not to check a program's modification slot when replenishing + /// a program cache instance. + pub check_program_modification_slot: bool, /// The compute budget to use for transaction execution. pub compute_budget: Option, /// The maximum number of bytes that log messages can consume. @@ -258,6 +261,7 @@ impl TransactionBatchProcessor { let program_cache_for_tx_batch = Rc::new(RefCell::new(self.replenish_program_cache( callbacks, &program_accounts_map, + config.check_program_modification_slot, config.limit_to_load_programs, ))); @@ -520,16 +524,22 @@ impl TransactionBatchProcessor { &self, callback: &CB, program_accounts_map: &HashMap, + check_program_modification_slot: bool, limit_to_load_programs: bool, ) -> ProgramCacheForTxBatch { let mut missing_programs: Vec<(Pubkey, (ProgramCacheMatchCriteria, u64))> = program_accounts_map .iter() .map(|(pubkey, count)| { - ( - *pubkey, - (callback.get_program_match_criteria(pubkey), *count), - ) + let match_criteria = if check_program_modification_slot { + get_program_modification_slot(callback, pubkey) + .map_or(ProgramCacheMatchCriteria::Tombstone, |slot| { + ProgramCacheMatchCriteria::DeployedOnOrAfterSlot(slot) + }) + } else { + ProgramCacheMatchCriteria::NoCriteria + }; + (*pubkey, (match_criteria, *count)) }) .collect(); @@ -1301,7 +1311,7 @@ mod tests { let mut account_maps: HashMap = HashMap::new(); account_maps.insert(key, 4); - batch_processor.replenish_program_cache(&mock_bank, &account_maps, true); + batch_processor.replenish_program_cache(&mock_bank, &account_maps, false, true); } #[test] @@ -1327,6 +1337,7 @@ mod tests { let result = batch_processor.replenish_program_cache( &mock_bank, &account_maps, + false, limit_to_load_programs, ); assert!(!result.hit_max_limit); @@ -1858,7 +1869,8 @@ mod tests { let maps = account_maps.clone(); let programs = programs.clone(); thread::spawn(move || { - let result = processor.replenish_program_cache(&local_bank, &maps, true); + let result = + processor.replenish_program_cache(&local_bank, &maps, false, true); for key in &programs { let cache_entry = result.find(key); assert!(matches!( diff --git a/svm/tests/conformance.rs b/svm/tests/conformance.rs index c3b96e383bb746..865cf325cb2596 100644 --- a/svm/tests/conformance.rs +++ b/svm/tests/conformance.rs @@ -279,6 +279,7 @@ fn run_fixture(fixture: InstrFixture, filename: OsString, execute_as_instr: bool }; let processor_config = TransactionProcessingConfig { account_overrides: None, + check_program_modification_slot: false, compute_budget: None, log_messages_bytes_limit: None, limit_to_load_programs: true,