Skip to content

Commit

Permalink
Add runtime environment to FailedVerification tombstones (#31991)
Browse files Browse the repository at this point in the history
* Add runtime environment to FailedVerification tombstones

* modify default variant

* prune update

* add DelayVisibility in prune_feature_set_transition

(cherry picked from commit c86e160)
  • Loading branch information
pgarg66 authored and mergify[bot] committed Jun 6, 2023
1 parent 859ca81 commit 0622fcc
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 18 deletions.
8 changes: 7 additions & 1 deletion ledger-tool/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,13 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
debug!("Failed to load program {}, error {:?}", key, err);
Arc::new(LoadedProgram::new_tombstone(
0,
LoadedProgramType::FailedVerification,
LoadedProgramType::FailedVerification(
bank.loaded_programs_cache
.read()
.unwrap()
.program_runtime_environment_v1
.clone(),
),
))
});
debug!("Loaded program {}", key);
Expand Down
33 changes: 20 additions & 13 deletions program-runtime/src/loaded_programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ pub trait WorkingSlot {
#[derive(Default)]
pub enum LoadedProgramType {
/// Tombstone for undeployed, closed or unloadable programs
FailedVerification(Arc<BuiltinProgram<InvokeContext<'static>>>),
#[default]
FailedVerification,
Closed,
DelayVisibility,
/// Successfully verified but not currently compiled, used to track usage statistics when a compiled program is evicted from memory.
Expand All @@ -77,7 +77,7 @@ pub enum LoadedProgramType {
impl Debug for LoadedProgramType {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
LoadedProgramType::FailedVerification => {
LoadedProgramType::FailedVerification(_) => {
write!(f, "LoadedProgramType::FailedVerification")
}
LoadedProgramType::Closed => write!(f, "LoadedProgramType::Closed"),
Expand Down Expand Up @@ -332,7 +332,7 @@ impl LoadedProgram {
pub fn is_tombstone(&self) -> bool {
matches!(
self.program,
LoadedProgramType::FailedVerification
LoadedProgramType::FailedVerification(_)
| LoadedProgramType::Closed
| LoadedProgramType::DelayVisibility
)
Expand Down Expand Up @@ -488,7 +488,9 @@ impl LoadedPrograms {
for second_level in self.entries.values_mut() {
second_level.retain(|entry| {
let retain = match &entry.program {
LoadedProgramType::Builtin(_) | LoadedProgramType::Closed => true,
LoadedProgramType::Builtin(_)
| LoadedProgramType::DelayVisibility
| LoadedProgramType::Closed => true,
LoadedProgramType::LegacyV0(program) | LoadedProgramType::LegacyV1(program)
if Arc::ptr_eq(
program.get_loader(),
Expand All @@ -498,6 +500,7 @@ impl LoadedPrograms {
true
}
LoadedProgramType::Unloaded(environment)
| LoadedProgramType::FailedVerification(environment)
if Arc::ptr_eq(environment, &self.program_runtime_environment_v1) =>
{
true
Expand Down Expand Up @@ -665,7 +668,7 @@ impl LoadedPrograms {
#[cfg(test)]
LoadedProgramType::TestLoaded(_) => Some((*id, program.clone())),
LoadedProgramType::Unloaded(_)
| LoadedProgramType::FailedVerification
| LoadedProgramType::FailedVerification(_)
| LoadedProgramType::Closed
| LoadedProgramType::DelayVisibility
| LoadedProgramType::Builtin(_) => None,
Expand Down Expand Up @@ -873,12 +876,13 @@ mod tests {
programs.push((program1, *deployment_slot, usage_counter));
});

let env = Arc::new(BuiltinProgram::new_loader(Config::default()));
for slot in 21..31 {
set_tombstone(
&mut cache,
program1,
slot,
LoadedProgramType::FailedVerification,
LoadedProgramType::FailedVerification(env.clone()),
);
}

Expand Down Expand Up @@ -959,7 +963,7 @@ mod tests {
matches!(
program_type,
LoadedProgramType::DelayVisibility
| LoadedProgramType::FailedVerification
| LoadedProgramType::FailedVerification(_)
| LoadedProgramType::Closed
)
});
Expand Down Expand Up @@ -1004,7 +1008,7 @@ mod tests {
matches!(
program_type,
LoadedProgramType::DelayVisibility
| LoadedProgramType::FailedVerification
| LoadedProgramType::FailedVerification(_)
| LoadedProgramType::Closed
)
});
Expand Down Expand Up @@ -1070,11 +1074,12 @@ mod tests {
fn test_replace_tombstones() {
let mut cache = LoadedPrograms::default();
let program1 = Pubkey::new_unique();
let env = Arc::new(BuiltinProgram::new_loader(Config::default()));
set_tombstone(
&mut cache,
program1,
10,
LoadedProgramType::FailedVerification,
LoadedProgramType::FailedVerification(env),
);

let loaded_program = new_test_loaded_program(10, 10);
Expand All @@ -1085,10 +1090,12 @@ mod tests {

#[test]
fn test_tombstone() {
let tombstone = LoadedProgram::new_tombstone(0, LoadedProgramType::FailedVerification);
let env = Arc::new(BuiltinProgram::new_loader(Config::default()));
let tombstone =
LoadedProgram::new_tombstone(0, LoadedProgramType::FailedVerification(env.clone()));
assert!(matches!(
tombstone.program,
LoadedProgramType::FailedVerification
LoadedProgramType::FailedVerification(_)
));
assert!(tombstone.is_tombstone());
assert_eq!(tombstone.deployment_slot, 0);
Expand All @@ -1106,7 +1113,7 @@ mod tests {
&mut cache,
program1,
10,
LoadedProgramType::FailedVerification,
LoadedProgramType::FailedVerification(env.clone()),
);
let second_level = &cache
.entries
Expand Down Expand Up @@ -1135,7 +1142,7 @@ mod tests {
&mut cache,
program2,
60,
LoadedProgramType::FailedVerification,
LoadedProgramType::FailedVerification(env),
);
let second_level = &cache
.entries
Expand Down
2 changes: 1 addition & 1 deletion programs/bpf_loader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ fn process_instruction_inner(

executor.ix_usage_counter.fetch_add(1, Ordering::Relaxed);
match &executor.program {
LoadedProgramType::FailedVerification
LoadedProgramType::FailedVerification(_)
| LoadedProgramType::Closed
| LoadedProgramType::DelayVisibility => {
Err(Box::new(InstructionError::InvalidAccountData) as Box<dyn std::error::Error>)
Expand Down
2 changes: 1 addition & 1 deletion programs/loader-v4/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ pub fn process_instruction_inner(
.ix_usage_counter
.fetch_add(1, Ordering::Relaxed);
match &loaded_program.program {
LoadedProgramType::FailedVerification
LoadedProgramType::FailedVerification(_)
| LoadedProgramType::Closed
| LoadedProgramType::DelayVisibility => {
Err(Box::new(InstructionError::InvalidAccountData) as Box<dyn std::error::Error>)
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ impl Accounts {
) -> Result<AccountSharedData> {
// Check for tombstone
let result = match &program.program {
LoadedProgramType::FailedVerification | LoadedProgramType::Closed => {
LoadedProgramType::FailedVerification(_) | LoadedProgramType::Closed => {
Err(TransactionError::InvalidProgramForExecution)
}
LoadedProgramType::DelayVisibility => {
Expand Down
8 changes: 7 additions & 1 deletion runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4450,7 +4450,13 @@ impl Bank {
debug!("Failed to load program {}, error {:?}", key, err);
Arc::new(LoadedProgram::new_tombstone(
self.slot,
LoadedProgramType::FailedVerification,
LoadedProgramType::FailedVerification(
self.loaded_programs_cache
.read()
.unwrap()
.program_runtime_environment_v1
.clone(),
),
))
});
program.tx_usage_counter.store(*count, Ordering::Relaxed);
Expand Down

0 comments on commit 0622fcc

Please sign in to comment.