From 35a993160f1e74efd1d02c50aa4d3f913bc8eade Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 3 Apr 2023 13:48:01 -0700 Subject: [PATCH 1/5] Add expiration slot to loaded program cache entry --- program-runtime/src/loaded_programs.rs | 82 +++++++++++++++++++++++--- programs/bpf_loader/src/lib.rs | 1 + programs/loader-v3/src/lib.rs | 1 + 3 files changed, 75 insertions(+), 9 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index c9fad65c705344..528ca05f1af22e 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -93,6 +93,8 @@ pub struct LoadedProgram { pub deployment_slot: Slot, /// Slot in which this entry will become active (can be in the future) pub effective_slot: Slot, + /// Optional expiration slot for this entry, after which it is treated as non-existent + pub maybe_expiration_slot: Option, /// How often this entry was used pub usage_counter: AtomicU64, } @@ -133,6 +135,7 @@ impl LoadedProgram { loader: Arc>>, deployment_slot: Slot, effective_slot: Slot, + maybe_expiration_slot: Option, elf_bytes: &[u8], account_size: usize, use_jit: bool, @@ -178,6 +181,7 @@ impl LoadedProgram { deployment_slot, account_size, effective_slot, + maybe_expiration_slot, usage_counter: AtomicU64::new(0), program, }) @@ -192,17 +196,21 @@ impl LoadedProgram { deployment_slot, account_size: 0, effective_slot: deployment_slot.saturating_add(1), + maybe_expiration_slot: None, usage_counter: AtomicU64::new(0), program: LoadedProgramType::BuiltIn(program), } } pub fn new_tombstone(slot: Slot, reason: LoadedProgramType) -> Self { + let maybe_expiration_slot = + matches!(reason, LoadedProgramType::DelayVisibility).then_some(slot.saturating_add(1)); let tombstone = Self { program: reason, account_size: 0, deployment_slot: slot, effective_slot: slot, + maybe_expiration_slot, usage_counter: AtomicU64::default(), }; debug_assert!(tombstone.is_tombstone()); @@ -308,8 +316,18 @@ impl LoadedPrograms { .filter_map(|key| { if let Some(second_level) = self.entries.get(&key) { for entry in second_level.iter().rev() { - if working_slot.current_slot() >= entry.effective_slot - && working_slot.is_ancestor(entry.deployment_slot) + let expiration_slot = entry.maybe_expiration_slot.unwrap_or(u64::MAX); + let current_slot = working_slot.current_slot(); + if current_slot >= expiration_slot { + // Found an entry that's already expired. 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 + && (current_slot == entry.deployment_slot + || working_slot.is_ancestor(entry.deployment_slot)) { return Some((key, entry.clone())); } @@ -385,6 +403,7 @@ mod tests { account_size: 0, deployment_slot, effective_slot, + maybe_expiration_slot: None, usage_counter: AtomicU64::default(), }) } @@ -844,6 +863,7 @@ mod tests { account_size: 0, deployment_slot, effective_slot, + maybe_expiration_slot: None, usage_counter, }) } @@ -880,7 +900,7 @@ mod tests { let mut fork_graph = TestForkGraphSpecific::default(); fork_graph.insert_fork(&[0, 10, 20, 22]); - fork_graph.insert_fork(&[0, 5, 11, 15, 16]); + 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(); @@ -901,8 +921,8 @@ mod tests { let program4 = Pubkey::new_unique(); assert!(!cache.replenish(program4, new_test_loaded_program(0, 1)).0); assert!(!cache.replenish(program4, new_test_loaded_program(5, 6)).0); - // The following is a special case, where effective slot is 4 slots in the future - assert!(!cache.replenish(program4, new_test_loaded_program(15, 19)).0); + // The following is a special case, where effective slot is 3 slots in the future + assert!(!cache.replenish(program4, new_test_loaded_program(15, 18)).0); // Current fork graph // 0 @@ -933,7 +953,7 @@ mod tests { assert!(missing.contains(&program3)); // Testing fork 0 - 5 - 11 - 15 - 16 with current slot at 16 - let mut working_slot = TestWorkingSlot::new(16, &[0, 5, 11, 15, 16, 19, 23]); + 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(), @@ -947,8 +967,8 @@ mod tests { assert!(missing.contains(&program3)); - // Testing the same fork above, but current slot is now 19 (equal to effective slot of program4). - working_slot.update_slot(19); + // Testing the same fork above, but current slot is now 18 (equal to effective slot of program4). + working_slot.update_slot(18); let (found, missing) = cache.extract( &working_slot, vec![program1, program2, program3, program4].into_iter(), @@ -957,7 +977,7 @@ mod tests { assert!(match_slot(&found, &program1, 0)); assert!(match_slot(&found, &program2, 11)); - // The effective slot of program4 deployed in slot 15 is 19. So it should be usable in slot 19. + // The effective slot of program4 deployed in slot 15 is 18. So it should be usable in slot 18. assert!(match_slot(&found, &program4, 15)); assert!(missing.contains(&program3)); @@ -990,6 +1010,50 @@ mod tests { assert!(missing.contains(&program3)); + // The following is a special case, where there's an expiration slot + let test_program = Arc::new(LoadedProgram { + program: LoadedProgramType::DelayVisibility, + account_size: 0, + deployment_slot: 19, + effective_slot: 19, + maybe_expiration_slot: Some(21), + usage_counter: Default::default(), + }); + assert!(!cache.replenish(program4, test_program).0); + + // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 19 + 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(), + ); + + assert!(match_slot(&found, &program1, 0)); + assert!(match_slot(&found, &program2, 11)); + // Program4 deployed at slot 19 should not be expired yet + assert!(match_slot(&found, &program4, 19)); + + assert!(missing.contains(&program3)); + + // Testing fork 0 - 5 - 11 - 15 - 16 - 19 - 21 - 23 with current slot at 21 + // This would cause program4 deployed at slot 19 to be expired. + 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(), + ); + + assert!(match_slot(&found, &program1, 0)); + assert!(match_slot(&found, &program2, 11)); + + assert!(missing.contains(&program3)); + assert!(missing.contains(&program4)); + + // Remove the expired entry to let the rest of the test continue + cache.entries.get_mut(&program4).map(|programs| { + programs.pop(); + }); + cache.prune(&fork_graph, 5); // Fork graph after pruning diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index a2a4fb465ea39f..fd7659869c025a 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -130,6 +130,7 @@ pub fn load_program_from_bytes( loader, deployment_slot, effective_slot, + None, programdata, account_size, use_jit, diff --git a/programs/loader-v3/src/lib.rs b/programs/loader-v3/src/lib.rs index 474454c1af4b0a..646c90350650f5 100644 --- a/programs/loader-v3/src/lib.rs +++ b/programs/loader-v3/src/lib.rs @@ -115,6 +115,7 @@ pub fn load_program_from_account( Arc::new(loader), state.slot, state.slot.saturating_add(1), + None, programdata, program.get_data().len(), use_jit, From b8759a4941afb819ce18dd415277bb71a2459bcb Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 3 Apr 2023 15:37:41 -0700 Subject: [PATCH 2/5] fix sanity --- program-runtime/src/loaded_programs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 528ca05f1af22e..72938d5b18fe43 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -1017,7 +1017,7 @@ mod tests { deployment_slot: 19, effective_slot: 19, maybe_expiration_slot: Some(21), - usage_counter: Default::default(), + usage_counter: AtomicU64::default(), }); assert!(!cache.replenish(program4, test_program).0); From 1ccbeb50dc50a38790a3aea283e9f614fda32568 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Mon, 3 Apr 2023 15:51:17 -0700 Subject: [PATCH 3/5] clippy fixes --- 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 72938d5b18fe43..971cd30e1685b9 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -1050,9 +1050,9 @@ mod tests { assert!(missing.contains(&program4)); // Remove the expired entry to let the rest of the test continue - cache.entries.get_mut(&program4).map(|programs| { + if let Some(programs) = cache.entries.get_mut(&program4) { programs.pop(); - }); + } cache.prune(&fork_graph, 5); From 03955cbbf42b8000d233f0d8acb79ca0e39496d0 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 4 Apr 2023 10:17:45 -0700 Subject: [PATCH 4/5] address review comments --- program-runtime/src/loaded_programs.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 971cd30e1685b9..8550ea85940ba9 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -316,9 +316,12 @@ impl LoadedPrograms { .filter_map(|key| { if let Some(second_level) = self.entries.get(&key) { for entry in second_level.iter().rev() { - let expiration_slot = entry.maybe_expiration_slot.unwrap_or(u64::MAX); let current_slot = working_slot.current_slot(); - if current_slot >= expiration_slot { + if entry + .maybe_expiration_slot + .map(|expiration_slot| current_slot >= expiration_slot) + .unwrap_or(false) + { // Found an entry that's already expired. Any further entries in the list // are older than the current one. So treat the program as missing in the // cache and return early. From 1fbd74a6f8a5eabed304cfbb11091f018ab82811 Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Tue, 4 Apr 2023 14:25:21 -0700 Subject: [PATCH 5/5] check if expired program is on the same fork --- program-runtime/src/loaded_programs.rs | 32 ++++++++++++++------------ 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index 8550ea85940ba9..123b823c5aafad 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -317,22 +317,24 @@ impl LoadedPrograms { if let Some(second_level) = self.entries.get(&key) { for entry in second_level.iter().rev() { let current_slot = working_slot.current_slot(); - if entry - .maybe_expiration_slot - .map(|expiration_slot| current_slot >= expiration_slot) - .unwrap_or(false) + if current_slot == entry.deployment_slot + || working_slot.is_ancestor(entry.deployment_slot) { - // Found an entry that's already expired. 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 - && (current_slot == entry.deployment_slot - || working_slot.is_ancestor(entry.deployment_slot)) - { - return Some((key, entry.clone())); + if entry + .maybe_expiration_slot + .map(|expiration_slot| current_slot >= expiration_slot) + .unwrap_or(false) + { + // Found an entry that's already expired. 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())); + } } } }