From bb66192edf25cb1c057ceacd38a6fe3cf3e50b34 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 24 Apr 2023 11:59:13 -0700 Subject: [PATCH 1/6] Check program modification slots during cold start --- Cargo.lock | 1 + Cargo.toml | 1 + ledger/src/blockstore_processor.rs | 2 +- program-runtime/src/loaded_programs.rs | 97 ++++++++++++++++++++++---- programs/loader-v3/src/lib.rs | 2 +- runtime/Cargo.toml | 1 + runtime/src/bank.rs | 50 ++++++++++++- runtime/src/bank_forks.rs | 12 +++- 8 files changed, 147 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 06287d68630863..60ae1c3e025c13 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6707,6 +6707,7 @@ dependencies = [ "solana-config-program", "solana-frozen-abi 1.16.0", "solana-frozen-abi-macro 1.16.0", + "solana-loader-v3-program", "solana-logger 1.16.0", "solana-measure", "solana-metrics", diff --git a/Cargo.toml b/Cargo.toml index 0fe15a41e82c7e..26907ec0d7d249 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -319,6 +319,7 @@ solana-genesis-utils = { path = "genesis-utils", version = "=1.16.0" } solana-geyser-plugin-interface = { path = "geyser-plugin-interface", version = "=1.16.0" } solana-geyser-plugin-manager = { path = "geyser-plugin-manager", version = "=1.16.0" } solana-gossip = { path = "gossip", version = "=1.16.0" } +solana-loader-v3-program = { path = "programs/loader-v3", version = "=1.16.0" } solana-ledger = { path = "ledger", version = "=1.16.0" } solana-local-cluster = { path = "local-cluster", version = "=1.16.0" } solana-logger = { path = "logger", version = "=1.16.0" } diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index c0d3ca6d7494f7..c66250415238f0 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -1434,7 +1434,7 @@ fn load_frozen_forks( let mut progress = ConfirmationProgress::new(last_entry_hash); let mut m = Measure::start("process_single_slot"); - let bank = bank_forks.write().unwrap().insert(bank); + let bank = bank_forks.write().unwrap().insert_from_ledger(bank); if process_single_slot( blockstore, &bank, diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 7ddf4cd2d17e5e..212bcbd361353b 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -351,11 +351,11 @@ impl LoadedPrograms { pub fn extract( &self, working_slot: &S, - keys: impl Iterator, + keys: impl Iterator, ) -> (HashMap>, Vec) { let mut missing = Vec::new(); let found = keys - .filter_map(|key| { + .filter_map(|(key, modified_on_or_after_slot)| { if let Some(second_level) = self.entries.get(&key) { for entry in second_level.iter().rev() { let current_slot = working_slot.current_slot(); @@ -374,6 +374,14 @@ impl LoadedPrograms { return None; } + if entry.deployment_slot < modified_on_or_after_slot { + // Found an entry that was deployed earlier than the expected modification slot. + // Any further entries in the list are older than the current one. + // So treat the program as missing in the cache and return early. + missing.push(key); + return None; + } + if current_slot >= entry.effective_slot { return Some((key, entry.clone())); } @@ -1153,7 +1161,7 @@ mod tests { let working_slot = TestWorkingSlot::new(22, &[0, 10, 20, 22]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 20)); @@ -1166,7 +1174,7 @@ mod tests { let mut working_slot = TestWorkingSlot::new(16, &[0, 5, 11, 15, 16, 18, 19, 23]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1181,7 +1189,7 @@ mod tests { working_slot.update_slot(18); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1196,7 +1204,7 @@ mod tests { working_slot.update_slot(23); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1211,7 +1219,7 @@ mod tests { let working_slot = TestWorkingSlot::new(11, &[0, 5, 11, 15, 16]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1235,7 +1243,7 @@ mod tests { let working_slot = TestWorkingSlot::new(19, &[0, 5, 11, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1250,7 +1258,7 @@ mod tests { let working_slot = TestWorkingSlot::new(21, &[0, 5, 11, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1285,7 +1293,7 @@ mod tests { let working_slot = TestWorkingSlot::new(22, &[0, 10, 20, 22]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); // Since the fork was pruned, we should not find the entry deployed at slot 20. @@ -1299,7 +1307,7 @@ mod tests { let working_slot = TestWorkingSlot::new(27, &[0, 5, 11, 25, 27]); let (found, _missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1328,7 +1336,7 @@ mod tests { let working_slot = TestWorkingSlot::new(27, &[0, 5, 11, 25, 27]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3, program4].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1339,6 +1347,65 @@ mod tests { assert!(missing.contains(&program3)); } + #[test] + fn test_extract_using_deployment_slot() { + let mut cache = LoadedPrograms::default(); + + // Fork graph created for the test + // 0 + // / \ + // 10 5 + // | | + // 20 11 + // | | \ + // 22 15 25 + // | | + // 16 27 + // | + // 19 + // | + // 23 + + let mut fork_graph = TestForkGraphSpecific::default(); + fork_graph.insert_fork(&[0, 10, 20, 22]); + fork_graph.insert_fork(&[0, 5, 11, 15, 16, 19, 21, 23]); + fork_graph.insert_fork(&[0, 5, 11, 25, 27]); + + let program1 = Pubkey::new_unique(); + assert!(!cache.replenish(program1, new_test_loaded_program(0, 1)).0); + assert!(!cache.replenish(program1, new_test_loaded_program(20, 21)).0); + + let program2 = Pubkey::new_unique(); + assert!(!cache.replenish(program2, new_test_loaded_program(5, 6)).0); + assert!(!cache.replenish(program2, new_test_loaded_program(11, 12)).0); + + let program3 = Pubkey::new_unique(); + assert!(!cache.replenish(program3, new_test_loaded_program(25, 26)).0); + + // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 + let working_slot = TestWorkingSlot::new(12, &[0, 5, 11, 12, 15, 16, 18, 19, 21, 23]); + let (found, missing) = cache.extract( + &working_slot, + vec![(program1, 0), (program2, 0), (program3, 0)].into_iter(), + ); + + assert!(match_slot(&found, &program1, 0)); + assert!(match_slot(&found, &program2, 11)); + + assert!(missing.contains(&program3)); + + // Test the same fork, but request the program modified at a later slot than what's in the cache. + let (found, missing) = cache.extract( + &working_slot, + vec![(program1, 5), (program2, 5), (program3, 0)].into_iter(), + ); + + assert!(match_slot(&found, &program2, 11)); + + assert!(missing.contains(&program1)); + assert!(missing.contains(&program3)); + } + #[test] fn test_prune_expired() { let mut cache = LoadedPrograms::default(); @@ -1389,7 +1456,7 @@ mod tests { let working_slot = TestWorkingSlot::new(12, &[0, 5, 11, 12, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0)].into_iter(), ); // Program1 deployed at slot 11 should not be expired yet @@ -1403,7 +1470,7 @@ mod tests { let working_slot = TestWorkingSlot::new(15, &[0, 5, 11, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![program1, program2, program3].into_iter(), + vec![(program1, 0), (program2, 0), (program3, 0)].into_iter(), ); assert!(match_slot(&found, &program2, 11)); @@ -1462,7 +1529,7 @@ mod tests { cache.prune(&fork_graph, 10); let working_slot = TestWorkingSlot::new(20, &[0, 10, 20]); - let (found, _missing) = cache.extract(&working_slot, vec![program1].into_iter()); + let (found, _missing) = cache.extract(&working_slot, vec![(program1, 0)].into_iter()); // The cache should have the program deployed at slot 0 assert_eq!( diff --git a/programs/loader-v3/src/lib.rs b/programs/loader-v3/src/lib.rs index 4e1042a3e05d0b..5b6d66cbf8d009 100644 --- a/programs/loader-v3/src/lib.rs +++ b/programs/loader-v3/src/lib.rs @@ -37,7 +37,7 @@ use { }, }; -fn get_state(data: &[u8]) -> Result<&LoaderV3State, InstructionError> { +pub fn get_state(data: &[u8]) -> Result<&LoaderV3State, InstructionError> { unsafe { let data = data .get(0..LoaderV3State::program_data_offset()) diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 0792abd6272708..06d7d8cf6f2125 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -49,6 +49,7 @@ solana-compute-budget-program = { workspace = true } solana-config-program = { workspace = true } solana-frozen-abi = { workspace = true } solana-frozen-abi-macro = { workspace = true } +solana-loader-v3-program = { workspace = true } solana-measure = { workspace = true } solana-metrics = { workspace = true } solana-perf = { workspace = true } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 61151bdcf877aa..d2e1211e53a71c 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -274,6 +274,7 @@ pub struct BankRc { #[cfg(RUSTC_WITH_SPECIALIZATION)] use solana_frozen_abi::abi_example::AbiExample; +use solana_sdk::loader_v3; #[cfg(RUSTC_WITH_SPECIALIZATION)] impl AbiExample for BankRc { @@ -811,6 +812,7 @@ impl PartialEq for Bank { fee_structure: _, incremental_snapshot_persistence: _, loaded_programs_cache: _, + check_program_modification_slot: _, // Ignore new fields explicitly if they do not impact PartialEq. // Adding ".." will remove compile-time checks that if a new field // is added to the struct, this PartialEq is accordingly updated. @@ -1066,6 +1068,8 @@ pub struct Bank { pub loaded_programs_cache: Arc>, + pub check_program_modification_slot: bool, + /// true when the bank's freezing or destruction has completed bank_freeze_or_destruction_incremented: AtomicBool, } @@ -1286,6 +1290,7 @@ impl Bank { accounts_data_size_delta_off_chain: AtomicI64::new(0), fee_structure: FeeStructure::default(), loaded_programs_cache: Arc::>::default(), + check_program_modification_slot: false, }; bank.bank_created(); @@ -1584,6 +1589,7 @@ impl Bank { accounts_data_size_delta_off_chain: AtomicI64::new(0), fee_structure: parent.fee_structure.clone(), loaded_programs_cache: parent.loaded_programs_cache.clone(), + check_program_modification_slot: false, }; let (_, ancestors_time_us) = measure_us!({ @@ -1910,6 +1916,7 @@ impl Bank { accounts_data_size_delta_off_chain: AtomicI64::new(0), fee_structure: FeeStructure::default(), loaded_programs_cache: Arc::>::default(), + check_program_modification_slot: false, }; bank.bank_created(); @@ -4127,6 +4134,35 @@ impl Bank { let _ = self.executor_cache.write().unwrap().remove(pubkey); } + fn program_modification_slot(&self, pubkey: &Pubkey) -> Result { + let program = self + .get_account_with_fixed_root(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_with_fixed_root(&programdata_address) + .ok_or(TransactionError::ProgramAccountNotFound)?; + if let Ok(UpgradeableLoaderState::ProgramData { + slot, + upgrade_authority_address: _, + }) = programdata.state() + { + return Ok(slot); + } + } + return Err(TransactionError::ProgramAccountNotFound); + } else if loader_v3::check_id(program.owner()) { + let state = solana_loader_v3_program::get_state(program.data()) + .map_err(|_| TransactionError::ProgramAccountNotFound)?; + return Ok(state.slot); + } + Ok(0) + } + #[allow(dead_code)] // Preparation for BankExecutorCache rework fn load_program(&self, pubkey: &Pubkey) -> Result> { let program = if let Some(program) = self.get_account_with_fixed_root(pubkey) { @@ -4403,10 +4439,22 @@ impl Bank { &self, program_accounts_map: &HashMap, ) -> HashMap> { + let programs_and_slots: Vec<(Pubkey, Slot)> = if self.check_program_modification_slot { + program_accounts_map + .keys() + .map(|pubkey| (*pubkey, self.program_modification_slot(pubkey).unwrap_or(0))) + .collect() + } else { + program_accounts_map + .keys() + .map(|pubkey| (*pubkey, 0 as Slot)) + .collect() + }; + let (mut loaded_programs_for_txs, missing_programs) = { // Lock the global cache to figure out which programs need to be loaded let loaded_programs_cache = self.loaded_programs_cache.read().unwrap(); - loaded_programs_cache.extract(self, program_accounts_map.keys().cloned()) + loaded_programs_cache.extract(self, programs_and_slots.into_iter()) }; // Load missing programs while global cache is unlocked diff --git a/runtime/src/bank_forks.rs b/runtime/src/bank_forks.rs index 017d9db40e3c47..9dc5d2750a8057 100644 --- a/runtime/src/bank_forks.rs +++ b/runtime/src/bank_forks.rs @@ -66,6 +66,7 @@ pub struct BankForks { pub accounts_hash_interval_slots: Slot, last_accounts_hash_slot: Slot, in_vote_only_mode: Arc, + highest_slot_at_startup: Slot, } impl Index for BankForks { @@ -182,10 +183,14 @@ impl BankForks { accounts_hash_interval_slots: std::u64::MAX, last_accounts_hash_slot: root, in_vote_only_mode: Arc::new(AtomicBool::new(false)), + highest_slot_at_startup: 0, } } - pub fn insert(&mut self, bank: Bank) -> Arc { + pub fn insert(&mut self, mut bank: Bank) -> Arc { + bank.check_program_modification_slot = + self.root.load(Ordering::Relaxed) < self.highest_slot_at_startup; + let bank = Arc::new(bank); let prev = self.banks.insert(bank.slot(), bank.clone()); assert!(prev.is_none()); @@ -197,6 +202,11 @@ impl BankForks { bank } + pub fn insert_from_ledger(&mut self, bank: Bank) -> Arc { + self.highest_slot_at_startup = std::cmp::max(self.highest_slot_at_startup, bank.slot()); + self.insert(bank) + } + pub fn remove(&mut self, slot: Slot) -> Option> { let bank = self.banks.remove(&slot)?; for parent in bank.proper_ancestors() { From f76cec9805e520fda79fd9ffafdc727059ca0d94 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 24 Apr 2023 12:11:46 -0700 Subject: [PATCH 2/6] update sbf Cargo.lock --- programs/sbf/Cargo.lock | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index f3e1e5ce9bd683..bbe05b0dcd76d8 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5116,6 +5116,18 @@ dependencies = [ "trees", ] +[[package]] +name = "solana-loader-v3-program" +version = "1.16.0" +dependencies = [ + "log", + "rand 0.7.3", + "solana-measure", + "solana-program-runtime", + "solana-sdk 1.16.0", + "solana_rbpf", +] + [[package]] name = "solana-logger" version = "1.15.2" @@ -5611,6 +5623,7 @@ dependencies = [ "solana-config-program", "solana-frozen-abi 1.16.0", "solana-frozen-abi-macro 1.16.0", + "solana-loader-v3-program", "solana-measure", "solana-metrics", "solana-perf", From 1f5de6e1ace5f9d448666c1d9cdaddd9f78de4ae Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 24 Apr 2023 13:08:31 -0700 Subject: [PATCH 3/6] fix use clause --- runtime/src/bank.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d2e1211e53a71c..cd7a885a875fd8 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -136,6 +136,7 @@ use { inflation::Inflation, instruction::{CompiledInstruction, TRANSACTION_LEVEL_STACK_HEIGHT}, lamports::LamportsError, + loader_v3, message::{AccountKeys, SanitizedMessage}, native_loader, native_token::{sol_to_lamports, LAMPORTS_PER_SOL}, @@ -274,7 +275,6 @@ pub struct BankRc { #[cfg(RUSTC_WITH_SPECIALIZATION)] use solana_frozen_abi::abi_example::AbiExample; -use solana_sdk::loader_v3; #[cfg(RUSTC_WITH_SPECIALIZATION)] impl AbiExample for BankRc { @@ -4442,7 +4442,14 @@ impl Bank { let programs_and_slots: Vec<(Pubkey, Slot)> = if self.check_program_modification_slot { program_accounts_map .keys() - .map(|pubkey| (*pubkey, self.program_modification_slot(pubkey).unwrap_or(0))) + .map(|pubkey| { + ( + *pubkey, + // If the program_modification_slot() returns error, use maximum slot + // number. It'll prevent cache from returning an old/obsolete program. + self.program_modification_slot(pubkey).unwrap_or(u64::MAX), + ) + }) .collect() } else { program_accounts_map From 42ac95a7824c2fae0d21b9c850519b3ff9a48d54 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 25 Apr 2023 07:25:04 -0700 Subject: [PATCH 4/6] improve code --- program-runtime/src/loaded_programs.rs | 150 +++++++++++++++++++++---- runtime/src/bank.rs | 39 ++++--- 2 files changed, 150 insertions(+), 39 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 212bcbd361353b..4824a6ea72fb9f 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -272,6 +272,12 @@ impl solana_frozen_abi::abi_example::AbiExample for LoadedPrograms { } } +pub enum LoadedProgramMatchCriteria { + DeployedOnOrAfterSlot(Slot), + Closed, + NoCriteria, +} + impl LoadedPrograms { /// Refill the cache with a single entry. It's typically called during transaction loading, /// when the cache doesn't contain the entry corresponding to program `key`. @@ -346,16 +352,31 @@ impl LoadedPrograms { self.remove_programs_with_no_entries(); } + fn matches_loaded_program( + program: &Arc, + criteria: &LoadedProgramMatchCriteria, + ) -> bool { + match criteria { + LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(slot) => { + program.deployment_slot >= *slot + } + LoadedProgramMatchCriteria::Closed => { + matches!(program.program, LoadedProgramType::Closed) + } + LoadedProgramMatchCriteria::NoCriteria => true, + } + } + /// Extracts a subset of the programs relevant to a transaction batch /// and returns which program accounts the accounts DB needs to load. pub fn extract( &self, working_slot: &S, - keys: impl Iterator, + keys: impl Iterator, ) -> (HashMap>, Vec) { let mut missing = Vec::new(); let found = keys - .filter_map(|(key, modified_on_or_after_slot)| { + .filter_map(|(key, match_criteria)| { if let Some(second_level) = self.entries.get(&key) { for entry in second_level.iter().rev() { let current_slot = working_slot.current_slot(); @@ -374,10 +395,7 @@ impl LoadedPrograms { return None; } - if entry.deployment_slot < modified_on_or_after_slot { - // Found an entry that was deployed earlier than the expected modification slot. - // Any further entries in the list are older than the current one. - // So treat the program as missing in the cache and return early. + if !Self::matches_loaded_program(entry, &match_criteria) { missing.push(key); return None; } @@ -507,6 +525,7 @@ impl LoadedPrograms { #[cfg(test)] mod tests { + use crate::loaded_programs::LoadedProgramMatchCriteria; use { crate::loaded_programs::{ BlockRelation, ForkGraph, LoadedProgram, LoadedProgramType, LoadedPrograms, WorkingSlot, @@ -1161,7 +1180,13 @@ mod tests { let working_slot = TestWorkingSlot::new(22, &[0, 10, 20, 22]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 20)); @@ -1174,7 +1199,13 @@ mod tests { let mut working_slot = TestWorkingSlot::new(16, &[0, 5, 11, 15, 16, 18, 19, 23]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1189,7 +1220,13 @@ mod tests { working_slot.update_slot(18); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1204,7 +1241,13 @@ mod tests { working_slot.update_slot(23); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1219,7 +1262,13 @@ mod tests { let working_slot = TestWorkingSlot::new(11, &[0, 5, 11, 15, 16]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1243,7 +1292,13 @@ mod tests { let working_slot = TestWorkingSlot::new(19, &[0, 5, 11, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1258,7 +1313,13 @@ mod tests { let working_slot = TestWorkingSlot::new(21, &[0, 5, 11, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1293,7 +1354,13 @@ mod tests { let working_slot = TestWorkingSlot::new(22, &[0, 10, 20, 22]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); // Since the fork was pruned, we should not find the entry deployed at slot 20. @@ -1307,7 +1374,13 @@ mod tests { let working_slot = TestWorkingSlot::new(27, &[0, 5, 11, 25, 27]); let (found, _missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1336,7 +1409,13 @@ mod tests { let working_slot = TestWorkingSlot::new(27, &[0, 5, 11, 25, 27]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0), (program4, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + (program4, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1386,7 +1465,12 @@ mod tests { let working_slot = TestWorkingSlot::new(12, &[0, 5, 11, 12, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program1, 0)); @@ -1397,7 +1481,18 @@ mod tests { // Test the same fork, but request the program modified at a later slot than what's in the cache. let (found, missing) = cache.extract( &working_slot, - vec![(program1, 5), (program2, 5), (program3, 0)].into_iter(), + vec![ + ( + program1, + LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(5), + ), + ( + program2, + LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(5), + ), + (program3, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program2, 11)); @@ -1456,7 +1551,12 @@ mod tests { let working_slot = TestWorkingSlot::new(12, &[0, 5, 11, 12, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); // Program1 deployed at slot 11 should not be expired yet @@ -1470,7 +1570,12 @@ mod tests { let working_slot = TestWorkingSlot::new(15, &[0, 5, 11, 15, 16, 18, 19, 21, 23]); let (found, missing) = cache.extract( &working_slot, - vec![(program1, 0), (program2, 0), (program3, 0)].into_iter(), + vec![ + (program1, LoadedProgramMatchCriteria::NoCriteria), + (program2, LoadedProgramMatchCriteria::NoCriteria), + (program3, LoadedProgramMatchCriteria::NoCriteria), + ] + .into_iter(), ); assert!(match_slot(&found, &program2, 11)); @@ -1529,7 +1634,10 @@ mod tests { cache.prune(&fork_graph, 10); let working_slot = TestWorkingSlot::new(20, &[0, 10, 20]); - let (found, _missing) = cache.extract(&working_slot, vec![(program1, 0)].into_iter()); + let (found, _missing) = cache.extract( + &working_slot, + vec![(program1, LoadedProgramMatchCriteria::NoCriteria)].into_iter(), + ); // The cache should have the program deployed at slot 0 assert_eq!( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index cd7a885a875fd8..c5d2267651653e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -275,6 +275,7 @@ pub struct BankRc { #[cfg(RUSTC_WITH_SPECIALIZATION)] use solana_frozen_abi::abi_example::AbiExample; +use solana_program_runtime::loaded_programs::LoadedProgramMatchCriteria; #[cfg(RUSTC_WITH_SPECIALIZATION)] impl AbiExample for BankRc { @@ -4439,24 +4440,26 @@ impl Bank { &self, program_accounts_map: &HashMap, ) -> HashMap> { - let programs_and_slots: Vec<(Pubkey, Slot)> = if self.check_program_modification_slot { - program_accounts_map - .keys() - .map(|pubkey| { - ( - *pubkey, - // If the program_modification_slot() returns error, use maximum slot - // number. It'll prevent cache from returning an old/obsolete program. - self.program_modification_slot(pubkey).unwrap_or(u64::MAX), - ) - }) - .collect() - } else { - program_accounts_map - .keys() - .map(|pubkey| (*pubkey, 0 as Slot)) - .collect() - }; + let programs_and_slots: Vec<(Pubkey, LoadedProgramMatchCriteria)> = + if self.check_program_modification_slot { + program_accounts_map + .keys() + .map(|pubkey| { + ( + *pubkey, + self.program_modification_slot(pubkey) + .map_or(LoadedProgramMatchCriteria::Closed, |slot| { + LoadedProgramMatchCriteria::DeployedOnOrAfterSlot(slot) + }), + ) + }) + .collect() + } else { + program_accounts_map + .keys() + .map(|pubkey| (*pubkey, LoadedProgramMatchCriteria::NoCriteria)) + .collect() + }; let (mut loaded_programs_for_txs, missing_programs) = { // Lock the global cache to figure out which programs need to be loaded From 0045969d3eff4582e028c9af1454dc71cb568598 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 25 Apr 2023 07:31:00 -0700 Subject: [PATCH 5/6] fix clippy --- program-runtime/src/loaded_programs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 4824a6ea72fb9f..66a28d60db35d9 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -525,10 +525,10 @@ impl LoadedPrograms { #[cfg(test)] mod tests { - use crate::loaded_programs::LoadedProgramMatchCriteria; use { crate::loaded_programs::{ - BlockRelation, ForkGraph, LoadedProgram, LoadedProgramType, LoadedPrograms, WorkingSlot, + BlockRelation, ForkGraph, LoadedProgram, LoadedProgramMatchCriteria, LoadedProgramType, + LoadedPrograms, WorkingSlot, }, percentage::Percentage, solana_rbpf::vm::BuiltInProgram, From 7ca824fda786ba759888c9bea9642d8516b17bac Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Wed, 26 Apr 2023 14:06:07 -0700 Subject: [PATCH 6/6] explicit else clause --- runtime/src/bank.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c5d2267651653e..7e5812523ae8ff 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4155,13 +4155,14 @@ impl Bank { return Ok(slot); } } - return Err(TransactionError::ProgramAccountNotFound); + Err(TransactionError::ProgramAccountNotFound) } else if loader_v3::check_id(program.owner()) { let state = solana_loader_v3_program::get_state(program.data()) .map_err(|_| TransactionError::ProgramAccountNotFound)?; - return Ok(state.slot); + Ok(state.slot) + } else { + Ok(0) } - Ok(0) } #[allow(dead_code)] // Preparation for BankExecutorCache rework