diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 293214a870f13a..a60d08b18cbfe2 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -193,7 +193,7 @@ pub struct InvokeContext<'a> { /// Information about the currently executing transaction. pub transaction_context: &'a mut TransactionContext, /// The local program cache for the transaction batch. - pub program_cache_for_tx_batch: &'a ProgramCacheForTxBatch, + pub program_cache_for_tx_batch: &'a mut ProgramCacheForTxBatch, /// Runtime configurations used to provision the invocation environment. pub environment_config: EnvironmentConfig<'a>, /// The compute budget for the current invocation. @@ -202,7 +202,6 @@ pub struct InvokeContext<'a> { /// the designated compute budget during program execution. compute_meter: RefCell, log_collector: Option>>, - pub programs_modified_by_tx: &'a mut ProgramCacheForTxBatch, /// Latest measurement not yet accumulated in [ExecuteDetailsTimings::execute_us] pub execute_time: Option, pub timings: ExecuteDetailsTimings, @@ -214,11 +213,10 @@ impl<'a> InvokeContext<'a> { #[allow(clippy::too_many_arguments)] pub fn new( transaction_context: &'a mut TransactionContext, - program_cache_for_tx_batch: &'a ProgramCacheForTxBatch, + program_cache_for_tx_batch: &'a mut ProgramCacheForTxBatch, environment_config: EnvironmentConfig<'a>, log_collector: Option>>, compute_budget: ComputeBudget, - programs_modified_by_tx: &'a mut ProgramCacheForTxBatch, ) -> Self { Self { transaction_context, @@ -227,7 +225,6 @@ impl<'a> InvokeContext<'a> { log_collector, compute_budget, compute_meter: RefCell::new(compute_budget.compute_unit_limit), - programs_modified_by_tx, execute_time: None, timings: ExecuteDetailsTimings::default(), syscall_context: Vec::new(), @@ -235,14 +232,6 @@ impl<'a> InvokeContext<'a> { } } - pub fn find_program_in_cache(&self, pubkey: &Pubkey) -> Option> { - // First lookup the cache of the programs modified by the current transaction. If not found, lookup - // the cache of the cache of the programs that are loaded for the transaction batch. - self.programs_modified_by_tx - .find(pubkey) - .or_else(|| self.program_cache_for_tx_batch.find(pubkey)) - } - pub fn get_environments_for_slot( &self, effective_slot: Slot, @@ -733,15 +722,13 @@ macro_rules! with_mock_invoke_context { 0, &sysvar_cache, ); - let program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); + let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); let mut $invoke_context = InvokeContext::new( &mut $transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, environment_config, Some(LogCollector::new_ref()), compute_budget, - &mut programs_modified_by_tx, ); }; } @@ -802,7 +789,7 @@ pub fn mock_process_instruction>, + /// Program entries modified during the transaction batch. + modified_entries: HashMap>, slot: Slot, pub environments: ProgramRuntimeEnvironments, /// Anticipated replacement for `environments` at the next epoch. @@ -689,6 +691,7 @@ impl ProgramCacheForTxBatch { ) -> Self { Self { entries: HashMap::new(), + modified_entries: HashMap::new(), slot, environments, upcoming_environments, @@ -706,6 +709,7 @@ impl ProgramCacheForTxBatch { ) -> Self { Self { entries: HashMap::new(), + modified_entries: HashMap::new(), slot, environments: cache.get_environments_for_epoch(epoch), upcoming_environments: cache.get_upcoming_environments_for_epoch(epoch), @@ -716,14 +720,6 @@ impl ProgramCacheForTxBatch { } } - pub fn entries(&self) -> &HashMap> { - &self.entries - } - - pub fn take_entries(&mut self) -> HashMap> { - std::mem::take(&mut self.entries) - } - /// Returns the current environments depending on the given epoch pub fn get_environments_for_epoch(&self, epoch: Epoch) -> &ProgramRuntimeEnvironments { if epoch != self.latest_root_epoch { @@ -747,21 +743,39 @@ impl ProgramCacheForTxBatch { (self.entries.insert(key, entry.clone()).is_some(), entry) } + /// Store an entry in `modified_entries` for a program modified during the + /// transaction batch. + pub fn store_modified_entry(&mut self, key: Pubkey, entry: Arc) { + self.modified_entries.insert(key, entry); + } + + /// Drain the program cache's modified entries, returning the owned + /// collection. + pub fn drain_modified_entries(&mut self) -> HashMap> { + std::mem::take(&mut self.modified_entries) + } + pub fn find(&self, key: &Pubkey) -> Option> { - self.entries.get(key).map(|entry| { - if entry.is_implicit_delay_visibility_tombstone(self.slot) { - // Found a program entry on the current fork, but it's not effective - // yet. It indicates that the program has delayed visibility. Return - // the tombstone to reflect that. - Arc::new(ProgramCacheEntry::new_tombstone( - entry.deployment_slot, - entry.account_owner, - ProgramCacheEntryType::DelayVisibility, - )) - } else { - entry.clone() - } - }) + // First lookup the cache of the programs modified by the current + // transaction. If not found, lookup the cache of the cache of the + // programs that are loaded for the transaction batch. + self.modified_entries + .get(key) + .or(self.entries.get(key)) + .map(|entry| { + if entry.is_implicit_delay_visibility_tombstone(self.slot) { + // Found a program entry on the current fork, but it's not effective + // yet. It indicates that the program has delayed visibility. Return + // the tombstone to reflect that. + Arc::new(ProgramCacheEntry::new_tombstone( + entry.deployment_slot, + entry.account_owner, + ProgramCacheEntryType::DelayVisibility, + )) + } else { + entry.clone() + } + }) } pub fn slot(&self) -> Slot { diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 8f14544ffcf0b3..dd8aa542213621 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -152,7 +152,7 @@ macro_rules! deploy_program { environments.program_runtime_v1.clone(), true, )?; - if let Some(old_entry) = $invoke_context.find_program_in_cache(&$program_id) { + if let Some(old_entry) = $invoke_context.program_cache_for_tx_batch.find(&$program_id) { executor.tx_usage_counter.store( old_entry.tx_usage_counter.load(Ordering::Relaxed), Ordering::Relaxed @@ -165,7 +165,7 @@ macro_rules! deploy_program { $drop load_program_metrics.program_id = $program_id.to_string(); load_program_metrics.submit_datapoint(&mut $invoke_context.timings); - $invoke_context.programs_modified_by_tx.replenish($program_id, Arc::new(executor)); + $invoke_context.program_cache_for_tx_batch.store_modified_entry($program_id, Arc::new(executor)); }}; } @@ -437,7 +437,8 @@ pub fn process_instruction_inner( let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time"); let executor = invoke_context - .find_program_in_cache(program_account.get_key()) + .program_cache_for_tx_batch + .find(program_account.get_key()) .ok_or_else(|| { ic_logger_msg!(log_collector, "Program is not cached"); InstructionError::InvalidAccountData @@ -1109,14 +1110,16 @@ fn process_loader_upgradeable_instruction( &log_collector, )?; let clock = invoke_context.get_sysvar_cache().get_clock()?; - invoke_context.programs_modified_by_tx.replenish( - program_key, - Arc::new(ProgramCacheEntry::new_tombstone( - clock.slot, - ProgramCacheEntryOwner::LoaderV3, - ProgramCacheEntryType::Closed, - )), - ); + invoke_context + .program_cache_for_tx_batch + .store_modified_entry( + program_key, + Arc::new(ProgramCacheEntry::new_tombstone( + clock.slot, + ProgramCacheEntryOwner::LoaderV3, + ProgramCacheEntryType::Closed, + )), + ); } _ => { ic_logger_msg!(log_collector, "Invalid Program account"); @@ -1543,11 +1546,11 @@ pub mod test_utils { false, ) { invoke_context - .programs_modified_by_tx + .program_cache_for_tx_batch .set_slot_for_tests(DELAY_VISIBILITY_SLOT_OFFSET); invoke_context - .programs_modified_by_tx - .replenish(*pubkey, Arc::new(loaded_program)); + .program_cache_for_tx_batch + .store_modified_entry(*pubkey, Arc::new(loaded_program)); } } } @@ -3763,7 +3766,7 @@ mod tests { latest_access_slot: AtomicU64::new(0), }; invoke_context - .programs_modified_by_tx + .program_cache_for_tx_batch .replenish(program_id, Arc::new(program)); assert_matches!( @@ -3772,7 +3775,7 @@ mod tests { ); let updated_program = invoke_context - .programs_modified_by_tx + .program_cache_for_tx_batch .find(&program_id) .expect("Didn't find upgraded program in the cache"); @@ -3807,7 +3810,7 @@ mod tests { latest_access_slot: AtomicU64::new(0), }; invoke_context - .programs_modified_by_tx + .program_cache_for_tx_batch .replenish(program_id, Arc::new(program)); let program_id2 = Pubkey::new_unique(); @@ -3817,7 +3820,7 @@ mod tests { ); let program2 = invoke_context - .programs_modified_by_tx + .program_cache_for_tx_batch .find(&program_id2) .expect("Didn't find upgraded program in the cache"); diff --git a/programs/loader-v4/src/lib.rs b/programs/loader-v4/src/lib.rs index b564e84c43a9d4..acf0255579d74e 100644 --- a/programs/loader-v4/src/lib.rs +++ b/programs/loader-v4/src/lib.rs @@ -449,7 +449,10 @@ pub fn process_instruction_deploy( state.slot = current_slot; state.status = LoaderV4Status::Deployed; - if let Some(old_entry) = invoke_context.find_program_in_cache(program.get_key()) { + if let Some(old_entry) = invoke_context + .program_cache_for_tx_batch + .find(program.get_key()) + { executor.tx_usage_counter.store( old_entry.tx_usage_counter.load(Ordering::Relaxed), Ordering::Relaxed, @@ -460,8 +463,8 @@ pub fn process_instruction_deploy( ); } invoke_context - .programs_modified_by_tx - .replenish(*program.get_key(), Arc::new(executor)); + .program_cache_for_tx_batch + .store_modified_entry(*program.get_key(), Arc::new(executor)); Ok(()) } @@ -592,7 +595,8 @@ pub fn process_instruction_inner( } let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time"); let loaded_program = invoke_context - .find_program_in_cache(program.get_key()) + .program_cache_for_tx_batch + .find(program.get_key()) .ok_or_else(|| { ic_logger_msg!(log_collector, "Program is not cached"); InstructionError::InvalidAccountData @@ -661,7 +665,7 @@ mod tests { if let Ok(loaded_program) = ProgramCacheEntry::new( &loader_v4::id(), invoke_context - .programs_modified_by_tx + .program_cache_for_tx_batch .environments .program_runtime_v2 .clone(), @@ -671,10 +675,12 @@ mod tests { account.data().len(), &mut load_program_metrics, ) { - invoke_context.programs_modified_by_tx.set_slot_for_tests(0); invoke_context - .programs_modified_by_tx - .replenish(*pubkey, Arc::new(loaded_program)); + .program_cache_for_tx_batch + .set_slot_for_tests(0); + invoke_context + .program_cache_for_tx_batch + .store_modified_entry(*pubkey, Arc::new(loaded_program)); } } } @@ -708,7 +714,7 @@ mod tests { Entrypoint::vm, |invoke_context| { invoke_context - .programs_modified_by_tx + .program_cache_for_tx_batch .environments .program_runtime_v2 = Arc::new(create_program_runtime_environment_v2( &ComputeBudget::default(), diff --git a/runtime/src/bank/builtins/core_bpf_migration/mod.rs b/runtime/src/bank/builtins/core_bpf_migration/mod.rs index 2ebe6c997844bc..54f1a8eabdfdd0 100644 --- a/runtime/src/bank/builtins/core_bpf_migration/mod.rs +++ b/runtime/src/bank/builtins/core_bpf_migration/mod.rs @@ -169,17 +169,11 @@ impl Bank { let elf = &programdata[progradata_metadata_size..]; // Set up the two `LoadedProgramsForTxBatch` instances, as if // processing a new transaction batch. - let program_cache_for_tx_batch = ProgramCacheForTxBatch::new_from_cache( + let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::new_from_cache( self.slot, self.epoch, &self.transaction_processor.program_cache.read().unwrap(), ); - let mut programs_modified = ProgramCacheForTxBatch::new( - self.slot, - program_cache_for_tx_batch.environments.clone(), - program_cache_for_tx_batch.upcoming_environments.clone(), - program_cache_for_tx_batch.latest_root_epoch, - ); // Configure a dummy `InvokeContext` from the runtime's current // environment, as well as the two `ProgramCacheForTxBatch` @@ -202,7 +196,7 @@ impl Bank { let mut dummy_invoke_context = InvokeContext::new( &mut dummy_transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, EnvironmentConfig::new( Hash::default(), None, @@ -213,7 +207,6 @@ impl Bank { ), None, compute_budget, - &mut programs_modified, ); solana_bpf_loader_program::direct_deploy_program( @@ -232,7 +225,7 @@ impl Bank { .program_cache .write() .unwrap() - .merge(programs_modified.entries()); + .merge(&program_cache_for_tx_batch.drain_modified_entries()); Ok(()) } diff --git a/svm/src/message_processor.rs b/svm/src/message_processor.rs index eb442a23266064..20161d401de858 100644 --- a/svm/src/message_processor.rs +++ b/svm/src/message_processor.rs @@ -275,7 +275,6 @@ mod tests { ]), )); let sysvar_cache = SysvarCache::default(); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); let environment_config = EnvironmentConfig::new( Hash::default(), None, @@ -286,11 +285,10 @@ mod tests { ); let mut invoke_context = InvokeContext::new( &mut transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, environment_config, None, ComputeBudget::default(), - &mut programs_modified_by_tx, ); let result = MessageProcessor::process_message( &message, @@ -331,7 +329,6 @@ mod tests { ), ]), )); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); let environment_config = EnvironmentConfig::new( Hash::default(), None, @@ -342,11 +339,10 @@ mod tests { ); let mut invoke_context = InvokeContext::new( &mut transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, environment_config, None, ComputeBudget::default(), - &mut programs_modified_by_tx, ); let result = MessageProcessor::process_message( &message, @@ -377,7 +373,6 @@ mod tests { ), ]), )); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); let environment_config = EnvironmentConfig::new( Hash::default(), None, @@ -388,11 +383,10 @@ mod tests { ); let mut invoke_context = InvokeContext::new( &mut transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, environment_config, None, ComputeBudget::default(), - &mut programs_modified_by_tx, ); let result = MessageProcessor::process_message( &message, @@ -514,7 +508,6 @@ mod tests { Some(transaction_context.get_key_of_account_at_index(0).unwrap()), )); let sysvar_cache = SysvarCache::default(); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); let environment_config = EnvironmentConfig::new( Hash::default(), None, @@ -525,11 +518,10 @@ mod tests { ); let mut invoke_context = InvokeContext::new( &mut transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, environment_config, None, ComputeBudget::default(), - &mut programs_modified_by_tx, ); let result = MessageProcessor::process_message( &message, @@ -555,7 +547,6 @@ mod tests { )], Some(transaction_context.get_key_of_account_at_index(0).unwrap()), )); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); let environment_config = EnvironmentConfig::new( Hash::default(), None, @@ -566,11 +557,10 @@ mod tests { ); let mut invoke_context = InvokeContext::new( &mut transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, environment_config, None, ComputeBudget::default(), - &mut programs_modified_by_tx, ); let result = MessageProcessor::process_message( &message, @@ -593,7 +583,6 @@ mod tests { )], Some(transaction_context.get_key_of_account_at_index(0).unwrap()), )); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); let environment_config = EnvironmentConfig::new( Hash::default(), None, @@ -604,11 +593,10 @@ mod tests { ); let mut invoke_context = InvokeContext::new( &mut transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, environment_config, None, ComputeBudget::default(), - &mut programs_modified_by_tx, ); let result = MessageProcessor::process_message( &message, @@ -692,7 +680,6 @@ mod tests { mock_program_id, Arc::new(ProgramCacheEntry::new_builtin(0, 0, MockBuiltin::vm)), ); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); let environment_config = EnvironmentConfig::new( Hash::default(), None, @@ -703,11 +690,10 @@ mod tests { ); let mut invoke_context = InvokeContext::new( &mut transaction_context, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, environment_config, None, ComputeBudget::default(), - &mut programs_modified_by_tx, ); let result = MessageProcessor::process_message( &message, diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index cc0cbcdac5775e..be39a212704cd5 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -339,7 +339,7 @@ impl TransactionBatchProcessor { compute_budget, &mut execute_timings, &mut error_metrics, - &program_cache_for_tx_batch.borrow(), + &mut program_cache_for_tx_batch.borrow_mut(), environment, config, ); @@ -722,7 +722,7 @@ impl TransactionBatchProcessor { compute_budget: ComputeBudget, execute_timings: &mut ExecuteTimings, error_metrics: &mut TransactionErrorMetrics, - program_cache_for_tx_batch: &ProgramCacheForTxBatch, + program_cache_for_tx_batch: &mut ProgramCacheForTxBatch, environment: &TransactionProcessingEnvironment, config: &TransactionProcessingConfig, ) -> TransactionExecutionResult { @@ -775,12 +775,6 @@ impl TransactionBatchProcessor { let lamports_per_signature = environment.lamports_per_signature; let mut executed_units = 0u64; - let mut programs_modified_by_tx = ProgramCacheForTxBatch::new( - self.slot, - program_cache_for_tx_batch.environments.clone(), - program_cache_for_tx_batch.upcoming_environments.clone(), - program_cache_for_tx_batch.latest_root_epoch, - ); let sysvar_cache = &self.sysvar_cache.read().unwrap(); let mut invoke_context = InvokeContext::new( @@ -796,7 +790,6 @@ impl TransactionBatchProcessor { ), log_collector.clone(), compute_budget, - &mut programs_modified_by_tx, ); let mut process_message_time = Measure::start("process_message_time"); @@ -903,7 +896,7 @@ impl TransactionBatchProcessor { executed_units, accounts_data_len_delta, }, - programs_modified_by_tx: programs_modified_by_tx.take_entries(), + programs_modified_by_tx: program_cache_for_tx_batch.drain_modified_entries(), } } @@ -1149,7 +1142,7 @@ mod tests { }; let sanitized_message = new_unchecked_sanitized_message(message); - let program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); + let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); let batch_processor = TransactionBatchProcessor::::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( @@ -1179,7 +1172,7 @@ mod tests { ComputeBudget::default(), &mut ExecuteTimings::default(), &mut TransactionErrorMetrics::default(), - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, &processing_environment, &processing_config, ); @@ -1201,7 +1194,7 @@ mod tests { ComputeBudget::default(), &mut ExecuteTimings::default(), &mut TransactionErrorMetrics::default(), - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, &processing_environment, &processing_config, ); @@ -1231,7 +1224,7 @@ mod tests { ComputeBudget::default(), &mut ExecuteTimings::default(), &mut TransactionErrorMetrics::default(), - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, &processing_environment, &processing_config, ); @@ -1271,7 +1264,7 @@ mod tests { }; let sanitized_message = new_unchecked_sanitized_message(message); - let program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); + let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::default(); let batch_processor = TransactionBatchProcessor::::default(); let sanitized_transaction = SanitizedTransaction::new_for_tests( @@ -1307,7 +1300,7 @@ mod tests { ComputeBudget::default(), &mut ExecuteTimings::default(), &mut error_metrics, - &program_cache_for_tx_batch, + &mut program_cache_for_tx_batch, &TransactionProcessingEnvironment::default(), &processing_config, ); diff --git a/svm/tests/conformance.rs b/svm/tests/conformance.rs index ec5bbe0c9fd529..fe4a7ac95e70ff 100644 --- a/svm/tests/conformance.rs +++ b/svm/tests/conformance.rs @@ -460,7 +460,6 @@ fn execute_fixture_as_instr( )), ); - let mut programs_modified_by_tx = ProgramCacheForTxBatch::default(); let log_collector = LogCollector::new_ref(); let sysvar_cache = &batch_processor.sysvar_cache.read().unwrap(); @@ -475,11 +474,10 @@ fn execute_fixture_as_instr( let mut invoke_context = InvokeContext::new( &mut transaction_context, - &loaded_programs, + &mut loaded_programs, env_config, Some(log_collector.clone()), compute_budget, - &mut programs_modified_by_tx, ); let mut instruction_accounts: Vec =