diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index 755c725837..fc87881d3b 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -15,9 +15,7 @@ use { }, solana_program_runtime::{ invoke_context::InvokeContext, - loaded_programs::{ - LoadProgramMetrics, LoadedProgram, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET, - }, + loaded_programs::{LoadProgramMetrics, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET}, with_mock_invoke_context, }, solana_rbpf::{ @@ -540,22 +538,8 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { let mut loaded_programs = LoadedProgramsForTxBatch::new(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET); for key in cached_account_keys { - let program = bank.load_program(&key).unwrap_or_else(|err| { - // Create a tombstone for the program in the cache - debug!("Failed to load program {}, error {:?}", key, err); - Arc::new(LoadedProgram::new_tombstone( - 0, - LoadedProgramType::FailedVerification( - bank.loaded_programs_cache - .read() - .unwrap() - .program_runtime_environment_v1 - .clone(), - ), - )) - }); + loaded_programs.replenish(key, bank.load_program(&key)); debug!("Loaded program {}", key); - loaded_programs.replenish(key, program); } invoke_context.programs_loaded_for_tx_batch = &loaded_programs; diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 3f1bcfa8da..304af92af6 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -26,16 +26,15 @@ use { solana_address_lookup_table_program::{error::AddressLookupError, state::AddressLookupTable}, solana_program_runtime::{ compute_budget::{self, ComputeBudget}, - loaded_programs::{LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch}, + loaded_programs::LoadedProgramsForTxBatch, }, solana_sdk::{ account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, account_utils::StateMut, - bpf_loader_upgradeable, + bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::{BankId, Slot}, feature_set::{ - self, add_set_tx_loaded_accounts_data_size_instruction, - delay_visibility_of_program_deployment, enable_request_heap_frame_ix, + self, add_set_tx_loaded_accounts_data_size_instruction, enable_request_heap_frame_ix, include_loaded_accounts_data_size_in_fee_calculation, remove_congestion_multiplier_from_fee_calculation, remove_deprecated_request_unit_ix, simplify_writable_program_account_check, use_default_units_in_fee_calculation, @@ -295,27 +294,8 @@ impl Accounts { fn account_shared_data_from_program( key: &Pubkey, - feature_set: &FeatureSet, - program: &LoadedProgram, program_accounts: &HashMap, ) -> Result { - // Check for tombstone - let result = match &program.program { - LoadedProgramType::FailedVerification(_) | LoadedProgramType::Closed => { - Err(TransactionError::InvalidProgramForExecution) - } - LoadedProgramType::DelayVisibility => { - debug_assert!(feature_set.is_active(&delay_visibility_of_program_deployment::id())); - Err(TransactionError::InvalidProgramForExecution) - } - _ => Ok(()), - }; - if feature_set.is_active(&simplify_writable_program_account_check::id()) { - // Currently CPI only fails if an execution is actually attempted. With this check it - // would also fail if a transaction just references an invalid program. So the checking - // of the result is being feature gated. - result?; - } // It's an executable program account. The program is already loaded in the cache. // So the account data is not needed. Return a dummy AccountSharedData with meta // information. @@ -389,9 +369,12 @@ impl Accounts { account_overrides.and_then(|overrides| overrides.get(key)) { (account_override.data().len(), account_override.clone(), 0) - } else if let Some(program) = (!instruction_account && !message.is_writable(i)) - .then_some(()) - .and_then(|_| loaded_programs.find(key)) + } else if let Some(program) = (feature_set + .is_active(&simplify_writable_program_account_check::id()) + && !instruction_account + && !message.is_writable(i)) + .then_some(()) + .and_then(|_| loaded_programs.find(key)) { // This condition block does special handling for accounts that are passed // as instruction account to any of the instructions in the transaction. @@ -399,13 +382,8 @@ impl Accounts { // (that are passed to the program as instruction accounts). So such accounts // are needed to be loaded even though corresponding compiled program may // already be present in the cache. - Self::account_shared_data_from_program( - key, - feature_set, - program.as_ref(), - program_accounts, - ) - .map(|program_account| (program.account_size, program_account, 0))? + Self::account_shared_data_from_program(key, program_accounts) + .map(|program_account| (program.account_size, program_account, 0))? } else { self.accounts_db .load_with_fixed_root(ancestors, key) @@ -461,18 +439,39 @@ impl Accounts { validated_fee_payer = true; } - if bpf_loader_upgradeable::check_id(account.owner()) { - if !feature_set.is_active(&simplify_writable_program_account_check::id()) - && message.is_writable(i) - && !message.is_upgradeable_loader_present() - { + if !feature_set.is_active(&simplify_writable_program_account_check::id()) { + if bpf_loader_upgradeable::check_id(account.owner()) { + if message.is_writable(i) && !message.is_upgradeable_loader_present() { + error_counters.invalid_writable_account += 1; + return Err(TransactionError::InvalidWritableAccount); + } + + if account.executable() { + // The upgradeable loader requires the derived ProgramData account + if let Ok(UpgradeableLoaderState::Program { + programdata_address, + }) = account.state() + { + if self + .accounts_db + .load_with_fixed_root(ancestors, &programdata_address) + .is_none() + { + error_counters.account_not_found += 1; + return Err(TransactionError::ProgramAccountNotFound); + } + } else { + error_counters.invalid_program_for_execution += 1; + return Err(TransactionError::InvalidProgramForExecution); + } + } + } else if account.executable() && message.is_writable(i) { error_counters.invalid_writable_account += 1; return Err(TransactionError::InvalidWritableAccount); } - } else if account.executable() && message.is_writable(i) { - error_counters.invalid_writable_account += 1; - return Err(TransactionError::InvalidWritableAccount); - } else if in_reward_interval + } + + if in_reward_interval && message.is_writable(i) && solana_stake_program::check_id(account.owner()) { @@ -1491,7 +1490,6 @@ mod tests { }, solana_sdk::{ account::{AccountSharedData, WritableAccount}, - bpf_loader_upgradeable::UpgradeableLoaderState, compute_budget::ComputeBudgetInstruction, epoch_schedule::EpochSchedule, genesis_config::ClusterType, @@ -2490,8 +2488,12 @@ mod tests { instructions, ); let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = - load_accounts_with_excluded_features(tx, &accounts, &mut error_counters, None); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[simplify_writable_program_account_check::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); @@ -2504,8 +2506,12 @@ mod tests { message.account_keys = vec![key0, key1, key2]; // revert key change message.header.num_readonly_unsigned_accounts = 2; // mark both executables as readonly let tx = Transaction::new(&[&keypair], message, Hash::default()); - let loaded_accounts = - load_accounts_with_excluded_features(tx, &accounts, &mut error_counters, None); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + Some(&[simplify_writable_program_account_check::id()]), + ); assert_eq!(error_counters.invalid_writable_account, 1); assert_eq!(loaded_accounts.len(), 1); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d37603ad51..8e840449ae 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4106,11 +4106,14 @@ impl Bank { } } - pub fn load_program(&self, pubkey: &Pubkey) -> Result> { + pub fn load_program(&self, pubkey: &Pubkey) -> Arc { let program = if let Some(program) = self.get_account_with_fixed_root(pubkey) { program } else { - return Err(TransactionError::ProgramAccountNotFound); + return Arc::new(LoadedProgram::new_tombstone( + self.slot, + LoadedProgramType::Closed, + )); }; let mut transaction_accounts = vec![(*pubkey, program)]; let is_upgradeable_loader = @@ -4125,10 +4128,16 @@ impl Bank { { transaction_accounts.push((programdata_address, programdata_account)); } else { - return Err(TransactionError::ProgramAccountNotFound); + return Arc::new(LoadedProgram::new_tombstone( + self.slot, + LoadedProgramType::Closed, + )); } } else { - return Err(TransactionError::ProgramAccountNotFound); + return Arc::new(LoadedProgram::new_tombstone( + self.slot, + LoadedProgramType::Closed, + )); } } let mut transaction_context = TransactionContext::new( @@ -4137,24 +4146,20 @@ impl Bank { 1, 1, ); - let instruction_context = transaction_context - .get_next_instruction_context() - .map_err(|err| TransactionError::InstructionError(0, err))?; + let instruction_context = transaction_context.get_next_instruction_context().unwrap(); instruction_context.configure(if is_upgradeable_loader { &[0, 1] } else { &[0] }, &[], &[]); - transaction_context - .push() - .map_err(|err| TransactionError::InstructionError(0, err))?; + transaction_context.push().unwrap(); let instruction_context = transaction_context .get_current_instruction_context() - .map_err(|err| TransactionError::InstructionError(0, err))?; + .unwrap(); let program = instruction_context .try_borrow_program_account(&transaction_context, 0) - .map_err(|err| TransactionError::InstructionError(0, err))?; + .unwrap(); let programdata = if is_upgradeable_loader { Some( instruction_context .try_borrow_program_account(&transaction_context, 1) - .map_err(|err| TransactionError::InstructionError(0, err))?, + .unwrap(), ) } else { None @@ -4170,14 +4175,19 @@ impl Bank { None, // log_collector &program, programdata.as_ref().unwrap_or(&program), - program_runtime_environment_v1, + program_runtime_environment_v1.clone(), ) .map(|(loaded_program, metrics)| { let mut timings = ExecuteDetailsTimings::default(); metrics.submit_datapoint(&mut timings); loaded_program }) - .map_err(|err| TransactionError::InstructionError(0, err)) + .unwrap_or_else(|_| { + Arc::new(LoadedProgram::new_tombstone( + self.slot, + LoadedProgramType::FailedVerification(program_runtime_environment_v1), + )) + }) } pub fn clear_program_cache(&self) { @@ -4423,20 +4433,7 @@ impl Bank { let missing_programs: Vec<(Pubkey, Arc)> = missing_programs .iter() .map(|(key, count)| { - let program = self.load_program(key).unwrap_or_else(|err| { - // Create a tombstone for the program in the cache - debug!("Failed to load program {}, error {:?}", key, err); - Arc::new(LoadedProgram::new_tombstone( - self.slot, - LoadedProgramType::FailedVerification( - self.loaded_programs_cache - .read() - .unwrap() - .program_runtime_environment_v1 - .clone(), - ), - )) - }); + let program = self.load_program(key); program.tx_usage_counter.store(*count, Ordering::Relaxed); (*key, program) }) diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 33a6581040..6e76b68571 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -7353,8 +7353,6 @@ fn test_bank_load_program() { bank.store_account_and_update_capitalization(&key1, &program_account); bank.store_account_and_update_capitalization(&programdata_key, &programdata_account); let program = bank.load_program(&key1); - assert!(program.is_ok()); - let program = program.unwrap(); assert!(matches!(program.program, LoadedProgramType::LegacyV1(_))); assert_eq!( program.account_size, @@ -7368,7 +7366,7 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { let mut bank = Bank::new_for_tests(&genesis_config); bank.feature_set = Arc::new(FeatureSet::all_enabled()); let bank = Arc::new(bank); - let bank_client = BankClient::new_shared(&bank); + let mut bank_client = BankClient::new_shared(&bank); // Setup keypairs and addresses let payer_keypair = Keypair::new(); @@ -7509,9 +7507,7 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { assert_eq!(*elf.get(i).unwrap(), *byte); } - let loaded_program = bank - .load_program(&program_keypair.pubkey()) - .expect("Failed to load the program"); + let loaded_program = bank.load_program(&program_keypair.pubkey()); // Invoke deployed program mock_process_instruction( @@ -7539,6 +7535,7 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() { // Test initialized program account bank.clear_signatures(); bank.store_account(&buffer_address, &buffer_account); + let bank = bank_client.advance_slot(1, &mint_keypair.pubkey()).unwrap(); let message = Message::new( &[Instruction::new_with_bincode( bpf_loader_upgradeable::id(),