Skip to content

Commit

Permalink
Escalates Option return value of load_program_accounts() to load_prog…
Browse files Browse the repository at this point in the history 
…ram_with_pubkey().
  • Loading branch information
@Lichtso
Lichtso committed Mar 29, 2024
1 parent 496b059 commit 71a38b3
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 79 deletions.
25 changes: 14 additions & 11 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1399,16 +1399,19 @@ impl Bank {
{
let effective_epoch = program_cache.latest_root_epoch.saturating_add(1);
drop(program_cache);
let recompiled = new.load_program(&key, false, effective_epoch);
recompiled
.tx_usage_counter
.fetch_add(program_to_recompile.tx_usage_counter.load(Relaxed), Relaxed);
recompiled
.ix_usage_counter
.fetch_add(program_to_recompile.ix_usage_counter.load(Relaxed), Relaxed);
let mut program_cache =
new.transaction_processor.program_cache.write().unwrap();
program_cache.assign_program(key, recompiled);
if let Some(recompiled) = new.load_program(&key, false, effective_epoch) {
recompiled.tx_usage_counter.fetch_add(
program_to_recompile.tx_usage_counter.load(Relaxed),
Relaxed,
);
recompiled.ix_usage_counter.fetch_add(
program_to_recompile.ix_usage_counter.load(Relaxed),
Relaxed,
);
let mut program_cache =
new.transaction_processor.program_cache.write().unwrap();
program_cache.assign_program(key, recompiled);
}
}
} else if new.epoch() != program_cache.latest_root_epoch
|| slot_index.saturating_add(slots_in_recompilation_phase) >= slots_in_epoch
Expand Down Expand Up @@ -7634,7 +7637,7 @@ impl Bank {
pubkey: &Pubkey,
reload: bool,
effective_epoch: Epoch,
) -> Arc<LoadedProgram> {
) -> Option<Arc<LoadedProgram>> {
self.transaction_processor
.load_program_with_pubkey(self, pubkey, reload, effective_epoch)
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/bank/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7145,7 +7145,7 @@ fn test_bank_load_program() {
programdata_account.set_rent_epoch(1);
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, false, bank.epoch());
let program = bank.load_program(&key1, false, bank.epoch()).unwrap();
assert_matches!(program.program, LoadedProgramType::LegacyV1(_));
assert_eq!(
program.account_size,
Expand Down
137 changes: 70 additions & 67 deletions svm/src/transaction_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,20 +385,21 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
pubkey: &Pubkey,
reload: bool,
effective_epoch: Epoch,
) -> Arc<LoadedProgram> {
) -> Option<Arc<LoadedProgram>> {
let program_cache = self.program_cache.read().unwrap();
let environments = program_cache.get_environments_for_epoch(effective_epoch);
let mut load_program_metrics = LoadProgramMetrics {
program_id: pubkey.to_string(),
..LoadProgramMetrics::default()
};

let mut loaded_program = match self.load_program_accounts(callbacks, pubkey) {
None | Some(ProgramAccountLoadResult::InvalidAccountData) => Ok(
LoadedProgram::new_tombstone(self.slot, LoadedProgramType::Closed),
),
let mut loaded_program = match self.load_program_accounts(callbacks, pubkey)? {
ProgramAccountLoadResult::InvalidAccountData => Ok(LoadedProgram::new_tombstone(
self.slot,
LoadedProgramType::Closed,
)),

Some(ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account)) => {
ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account) => {
Self::load_program_from_bytes(
&mut load_program_metrics,
program_account.data(),
Expand All @@ -411,11 +412,11 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
.map_err(|_| (0, environments.program_runtime_v1.clone()))
}

Some(ProgramAccountLoadResult::ProgramOfLoaderV3(
ProgramAccountLoadResult::ProgramOfLoaderV3(
program_account,
programdata_account,
slot,
)) => programdata_account
) => programdata_account
.data()
.get(UpgradeableLoaderState::size_of_programdata_metadata()..)
.ok_or(Box::new(InstructionError::InvalidAccountData).into())
Expand All @@ -435,24 +436,22 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
})
.map_err(|_| (slot, environments.program_runtime_v1.clone())),

Some(ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot)) => {
program_account
.data()
.get(LoaderV4State::program_data_offset()..)
.ok_or(Box::new(InstructionError::InvalidAccountData).into())
.and_then(|elf_bytes| {
Self::load_program_from_bytes(
&mut load_program_metrics,
elf_bytes,
&loader_v4::id(),
program_account.data().len(),
slot,
environments.program_runtime_v2.clone(),
reload,
)
})
.map_err(|_| (slot, environments.program_runtime_v2.clone()))
}
ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot) => program_account
.data()
.get(LoaderV4State::program_data_offset()..)
.ok_or(Box::new(InstructionError::InvalidAccountData).into())
.and_then(|elf_bytes| {
Self::load_program_from_bytes(
&mut load_program_metrics,
elf_bytes,
&loader_v4::id(),
program_account.data().len(),
slot,
environments.program_runtime_v2.clone(),
reload,
)
})
.map_err(|_| (slot, environments.program_runtime_v2.clone())),
}
.unwrap_or_else(|(slot, env)| {
LoadedProgram::new_tombstone(slot, LoadedProgramType::FailedVerification(env))
Expand All @@ -476,7 +475,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
.max(self.epoch_schedule.get_first_slot_in_epoch(effective_epoch));
}
loaded_program.update_access_slot(self.slot);
Arc::new(loaded_program)
Some(Arc::new(loaded_program))
}

fn replenish_program_cache<CB: TransactionProcessingCallback>(
Expand Down Expand Up @@ -541,7 +540,9 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {

if let Some((key, count)) = program_to_load {
// Load, verify and compile one program.
let program = self.load_program_with_pubkey(callback, &key, false, self.epoch);
let program = self
.load_program_with_pubkey(callback, &key, false, self.epoch)
.expect("called load_program_with_pubkey() with nonexistent account");
program.tx_usage_counter.store(count, Ordering::Relaxed);
program_to_store = Some((key, program));
} else if missing_programs.is_empty() {
Expand Down Expand Up @@ -1266,9 +1267,7 @@ mod tests {
let batch_processor = TransactionBatchProcessor::<TestForkGraph>::default();

let result = batch_processor.load_program_with_pubkey(&mock_bank, &key, false, 50);

let loaded_program = LoadedProgram::new_tombstone(0, LoadedProgramType::Closed);
assert_eq!(result, Arc::new(loaded_program));
assert!(result.is_none());
}

#[test]
Expand Down Expand Up @@ -1296,7 +1295,7 @@ mod tests {
.program_runtime_v1,
),
);
assert_eq!(result, Arc::new(loaded_program));
assert_eq!(result.unwrap(), Arc::new(loaded_program));
}

#[test]
Expand Down Expand Up @@ -1324,7 +1323,7 @@ mod tests {
.program_runtime_v1,
),
);
assert_eq!(result, Arc::new(loaded_program));
assert_eq!(result.unwrap(), Arc::new(loaded_program));

let buffer = load_test_program();
account_data.set_data(buffer);
Expand All @@ -1346,7 +1345,7 @@ mod tests {
false,
);

assert_eq!(result, Arc::new(expected.unwrap()));
assert_eq!(result.unwrap(), Arc::new(expected.unwrap()));
}

#[test]
Expand Down Expand Up @@ -1391,7 +1390,7 @@ mod tests {
.program_runtime_v1,
),
);
assert_eq!(result, Arc::new(loaded_program));
assert_eq!(result.unwrap(), Arc::new(loaded_program));

let mut buffer = load_test_program();
let mut header = bincode::serialize(&state).unwrap();
Expand Down Expand Up @@ -1426,7 +1425,7 @@ mod tests {
environments.program_runtime_v1.clone(),
false,
);
assert_eq!(result, Arc::new(expected.unwrap()));
assert_eq!(result.unwrap(), Arc::new(expected.unwrap()));
}

#[test]
Expand Down Expand Up @@ -1465,7 +1464,7 @@ mod tests {
.program_runtime_v1,
),
);
assert_eq!(result, Arc::new(loaded_program));
assert_eq!(result.unwrap(), Arc::new(loaded_program));

let mut header = account_data.data().to_vec();
let mut complement =
Expand Down Expand Up @@ -1498,7 +1497,7 @@ mod tests {
environments.program_runtime_v1.clone(),
false,
);
assert_eq!(result, Arc::new(expected.unwrap()));
assert_eq!(result.unwrap(), Arc::new(expected.unwrap()));
}

#[test]
Expand All @@ -1521,7 +1520,7 @@ mod tests {
let result = batch_processor.load_program_with_pubkey(&mock_bank, &key, false, 20);

let slot = batch_processor.epoch_schedule.get_first_slot_in_epoch(20);
assert_eq!(result.effective_slot, slot);
assert_eq!(result.unwrap().effective_slot, slot);
}

#[test]
Expand Down Expand Up @@ -1800,47 +1799,51 @@ mod tests {
assert_eq!(error_metrics.instruction_error, 1);
}

#[test]
#[should_panic = "called load_program_with_pubkey() with nonexistent account"]
fn test_replenish_program_cache_with_nonexistent_accounts() {
let mock_bank = MockBankCallback::default();
let batch_processor = TransactionBatchProcessor::<TestForkGraph>::default();
batch_processor.program_cache.write().unwrap().fork_graph =
Some(Arc::new(RwLock::new(TestForkGraph {})));
let key = Pubkey::new_unique();
let owner = Pubkey::new_unique();

let mut account_maps: HashMap<Pubkey, (&Pubkey, u64)> = HashMap::new();
account_maps.insert(key, (&owner, 4));

batch_processor.replenish_program_cache(&mock_bank, &account_maps, true);
}

#[test]
fn test_replenish_program_cache() {
// Case 1
let mut mock_bank = MockBankCallback::default();
let batch_processor = TransactionBatchProcessor::<TestForkGraph>::default();
batch_processor.program_cache.write().unwrap().fork_graph =
Some(Arc::new(RwLock::new(TestForkGraph {})));
let key1 = Pubkey::new_unique();
let key2 = Pubkey::new_unique();
let key = Pubkey::new_unique();
let owner = Pubkey::new_unique();

let mut account_data = AccountSharedData::default();
account_data.set_owner(bpf_loader::id());
mock_bank.account_shared_data.insert(key2, account_data);
mock_bank.account_shared_data.insert(key, account_data);

let mut account_maps: HashMap<Pubkey, (&Pubkey, u64)> = HashMap::new();
account_maps.insert(key1, (&owner, 2));
account_maps.insert(key, (&owner, 4));

account_maps.insert(key2, (&owner, 4));
let result = batch_processor.replenish_program_cache(&mock_bank, &account_maps, false);

let program1 = result.find(&key1).unwrap();
assert!(matches!(program1.program, LoadedProgramType::Closed));
assert!(!result.hit_max_limit);
let program2 = result.find(&key2).unwrap();
assert!(matches!(
program2.program,
LoadedProgramType::FailedVerification(_)
));

// Case 2
let result = batch_processor.replenish_program_cache(&mock_bank, &account_maps, true);

let program1 = result.find(&key1).unwrap();
assert!(matches!(program1.program, LoadedProgramType::Closed));
assert!(!result.hit_max_limit);
let program2 = result.find(&key2).unwrap();
assert!(matches!(
program2.program,
LoadedProgramType::FailedVerification(_)
));
for limit_to_load_programs in [false, true] {
let result = batch_processor.replenish_program_cache(
&mock_bank,
&account_maps,
limit_to_load_programs,
);
assert!(!result.hit_max_limit);
let program = result.find(&key).unwrap();
assert!(matches!(
program.program,
LoadedProgramType::FailedVerification(_)
));
}
}

#[test]
Expand Down

0 comments on commit 71a38b3

Please sign in to comment.