Skip to content

Commit

Permalink
v1.18: Fix - FailedVerification and Closed tombstones (backport of …
Browse files Browse the repository at this point in the history 
…#419) (#605)

Fix - `FailedVerification` and `Closed` tombstones (#419)

* Only the verifier can cause FailedVerification, everything else is Closed

* Removes the environments parameter from load_program_accounts().

* cargo fmt

* Simplify invocation of deployed program

* Attempt to invoke a program before it is deployed

* Attempt to invoke a buffer before it is used in a deployment

* Escalates Option return value of load_program_accounts() to load_program_with_pubkey().

* Review feedback

(cherry picked from commit 55c05c5)

Co-authored-by: Alexander Meißner <[email protected]>
  • Loading branch information
@mergify @Lichtso @yihau
2 people authored and yihau committed Apr 11, 2024
1 parent 8c8220f commit f2ea237
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 75 deletions.
6 changes: 5 additions & 1 deletion ledger-tool/src/program.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,11 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
let mut loaded_programs =
bank.new_program_cache_for_tx_batch_for_slot(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET);
for key in cached_account_keys {
loaded_programs.replenish(key, bank.load_program(&key, false, None));
loaded_programs.replenish(
key,
bank.load_program(&key, false, None)
.expect("Couldn't find program account"),
);
debug!("Loaded program {}", key);
}
invoke_context.programs_loaded_for_tx_batch = &loaded_programs;
Expand Down
13 changes: 12 additions & 1 deletion program-runtime/src/loaded_programs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,11 @@ pub trait ForkGraph {

#[derive(Default)]
pub enum LoadedProgramType {
/// Tombstone for undeployed, closed or unloadable programs
/// Tombstone for programs which currently do not pass the verifier but could if the feature set changed.
FailedVerification(ProgramRuntimeEnvironment),
/// Tombstone for programs that were either explicitly closed or never deployed.
///
/// It's also used for accounts belonging to program loaders, that don't actually contain program code (e.g. buffer accounts for LoaderV3 programs).
#[default]
Closed,
DelayVisibility,
Expand Down Expand Up @@ -1102,6 +1105,14 @@ impl<FG: ForkGraph> LoadedPrograms<FG> {
}
}

/// Returns the `slot_versions` of the second level for the given program id.
pub fn get_slot_versions_for_tests(&self, key: &Pubkey) -> &[Arc<LoadedProgram>] {
self.entries
.get(key)
.map(|second_level| second_level.slot_versions.as_ref())
.unwrap_or(&[])
}

fn unload_program(&mut self, id: &Pubkey) {
if let Some(second_level) = self.entries.get_mut(id) {
for entry in second_level.slot_versions.iter_mut() {
Expand Down
88 changes: 42 additions & 46 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ use {
loaded_programs::{
LoadProgramMetrics, LoadedProgram, LoadedProgramMatchCriteria, LoadedProgramType,
LoadedPrograms, LoadedProgramsForTxBatch, ProgramRuntimeEnvironment,
ProgramRuntimeEnvironments, DELAY_VISIBILITY_SLOT_OFFSET,
DELAY_VISIBILITY_SLOT_OFFSET,
},
log_collector::LogCollector,
message_processor::MessageProcessor,
Expand Down Expand Up @@ -308,8 +308,7 @@ impl BankRc {
}

enum ProgramAccountLoadResult {
AccountNotFound,
InvalidAccountData(ProgramRuntimeEnvironment),
InvalidAccountData,
ProgramOfLoaderV1orV2(AccountSharedData),
ProgramOfLoaderV3(AccountSharedData, AccountSharedData, Slot),
ProgramOfLoaderV4(AccountSharedData, Slot),
Expand Down Expand Up @@ -1375,9 +1374,12 @@ impl Bank {
loaded_programs_cache.programs_to_recompile.pop()
{
drop(loaded_programs_cache);
let recompiled = new.load_program(&key, false, Some(program_to_recompile));
let mut loaded_programs_cache = new.loaded_programs_cache.write().unwrap();
loaded_programs_cache.assign_program(key, recompiled);
if let Some(recompiled) =
new.load_program(&key, false, Some(program_to_recompile))
{
let mut loaded_programs_cache = new.loaded_programs_cache.write().unwrap();
loaded_programs_cache.assign_program(key, recompiled);
}
}
} else if new.epoch() != loaded_programs_cache.latest_root_epoch
|| slot_index.saturating_add(slots_in_recompilation_phase) >= slots_in_epoch
Expand Down Expand Up @@ -4556,58 +4558,52 @@ impl Bank {
}
}

fn load_program_accounts(
&self,
pubkey: &Pubkey,
environments: &ProgramRuntimeEnvironments,
) -> ProgramAccountLoadResult {
let program_account = match self.get_account_with_fixed_root(pubkey) {
None => return ProgramAccountLoadResult::AccountNotFound,
Some(account) => account,
};
fn load_program_accounts(&self, pubkey: &Pubkey) -> Option<ProgramAccountLoadResult> {
let program_account = self.get_account_with_fixed_root(pubkey)?;

debug_assert!(solana_bpf_loader_program::check_loader_id(
program_account.owner()
));

if loader_v4::check_id(program_account.owner()) {
return solana_loader_v4_program::get_state(program_account.data())
.ok()
.and_then(|state| {
(!matches!(state.status, LoaderV4Status::Retracted)).then_some(state.slot)
})
.map(|slot| ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot))
.unwrap_or(ProgramAccountLoadResult::InvalidAccountData(
environments.program_runtime_v2.clone(),
));
return Some(
solana_loader_v4_program::get_state(program_account.data())
.ok()
.and_then(|state| {
(!matches!(state.status, LoaderV4Status::Retracted)).then_some(state.slot)
})
.map(|slot| ProgramAccountLoadResult::ProgramOfLoaderV4(program_account, slot))
.unwrap_or(ProgramAccountLoadResult::InvalidAccountData),
);
}

if !bpf_loader_upgradeable::check_id(program_account.owner()) {
return ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account);
return Some(ProgramAccountLoadResult::ProgramOfLoaderV1orV2(
program_account,
));
}

if let Ok(UpgradeableLoaderState::Program {
programdata_address,
}) = program_account.state()
{
let programdata_account = match self.get_account_with_fixed_root(&programdata_address) {
None => return ProgramAccountLoadResult::AccountNotFound,
Some(account) => account,
};

if let Ok(UpgradeableLoaderState::ProgramData {
slot,
upgrade_authority_address: _,
}) = programdata_account.state()
if let Some(programdata_account) =
self.get_account_with_fixed_root(&programdata_address)
{
return ProgramAccountLoadResult::ProgramOfLoaderV3(
program_account,
programdata_account,
if let Ok(UpgradeableLoaderState::ProgramData {
slot,
);
upgrade_authority_address: _,
}) = programdata_account.state()
{
return Some(ProgramAccountLoadResult::ProgramOfLoaderV3(
program_account,
programdata_account,
slot,
));
}
}
}
ProgramAccountLoadResult::InvalidAccountData(environments.program_runtime_v1.clone())
Some(ProgramAccountLoadResult::InvalidAccountData)
}

fn load_program_from_bytes(
Expand Down Expand Up @@ -4652,7 +4648,7 @@ impl Bank {
pubkey: &Pubkey,
reload: bool,
recompile: Option<Arc<LoadedProgram>>,
) -> Arc<LoadedProgram> {
) -> Option<Arc<LoadedProgram>> {
let loaded_programs_cache = self.loaded_programs_cache.read().unwrap();
let effective_epoch = if recompile.is_some() {
loaded_programs_cache.latest_root_epoch.saturating_add(1)
Expand All @@ -4665,14 +4661,12 @@ impl Bank {
..LoadProgramMetrics::default()
};

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

ProgramAccountLoadResult::InvalidAccountData(env) => Err((self.slot, env)),

ProgramAccountLoadResult::ProgramOfLoaderV1orV2(program_account) => {
Self::load_program_from_bytes(
&mut load_program_metrics,
Expand Down Expand Up @@ -4755,7 +4749,7 @@ impl Bank {
AtomicU64::new(recompile.ix_usage_counter.load(Ordering::Relaxed));
}
loaded_program.update_access_slot(self.slot());
Arc::new(loaded_program)
Some(Arc::new(loaded_program))
}

pub fn clear_program_cache(&self) {
Expand Down Expand Up @@ -5013,7 +5007,9 @@ impl Bank {

if let Some((key, count)) = program_to_load {
// Load, verify and compile one program.
let program = self.load_program(&key, false, None);
let program = self
.load_program(&key, false, None)
.expect("called load_program() 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
79 changes: 52 additions & 27 deletions runtime/src/bank/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ use {
compute_budget::ComputeBudget,
compute_budget_processor::{self, MAX_COMPUTE_UNIT_LIMIT},
declare_process_instruction,
invoke_context::mock_process_instruction,
loaded_programs::{LoadedProgram, LoadedProgramType, DELAY_VISIBILITY_SLOT_OFFSET},
loaded_programs::{LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch},
prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType},
timings::ExecuteTimings,
},
Expand Down Expand Up @@ -7172,7 +7171,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, None);
let program = bank.load_program(&key1, false, None).unwrap();
assert_matches!(program.program, LoadedProgramType::LegacyV1(_));
assert_eq!(
program.account_size,
Expand All @@ -7198,6 +7197,26 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
);
let upgrade_authority_keypair = Keypair::new();

// Invoke not yet deployed program
let instruction = Instruction::new_with_bytes(program_keypair.pubkey(), &[], Vec::new());
let invocation_message = Message::new(&[instruction], Some(&mint_keypair.pubkey()));
let binding = mint_keypair.insecure_clone();
let transaction = Transaction::new(
&[&binding],
invocation_message.clone(),
bank.last_blockhash(),
);
assert_eq!(
bank.process_transaction(&transaction),
Err(TransactionError::ProgramAccountNotFound),
);
{
// Make sure it is not in the cache because the account owner is not a loader
let program_cache = bank.loaded_programs_cache.read().unwrap();
let slot_versions = program_cache.get_slot_versions_for_tests(&program_keypair.pubkey());
assert!(slot_versions.is_empty());
}

// Load program file
let mut file = File::open("../programs/bpf_loader/test_elfs/out/noop_aligned.so")
.expect("file open failed");
Expand Down Expand Up @@ -7245,6 +7264,28 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
&bpf_loader_upgradeable::id(),
);

// Test buffer invocation
bank.store_account(&buffer_address, &buffer_account);
let instruction = Instruction::new_with_bytes(buffer_address, &[], Vec::new());
let message = Message::new(&[instruction], Some(&mint_keypair.pubkey()));
let transaction = Transaction::new(&[&binding], message, bank.last_blockhash());
assert_eq!(
bank.process_transaction(&transaction),
Err(TransactionError::InstructionError(
0,
InstructionError::InvalidAccountData,
)),
);
{
let program_cache = bank.loaded_programs_cache.read().unwrap();
let slot_versions = program_cache.get_slot_versions_for_tests(&buffer_address);
assert_eq!(slot_versions.len(), 1);
assert!(matches!(
slot_versions[0].program,
LoadedProgramType::Closed,
));
}

// Test successful deploy
let payer_base_balance = LAMPORTS_PER_SOL;
let deploy_fees = {
Expand All @@ -7262,7 +7303,6 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
&system_program::id(),
),
);
bank.store_account(&buffer_address, &buffer_account);
bank.store_account(&program_keypair.pubkey(), &AccountSharedData::default());
bank.store_account(&programdata_address, &AccountSharedData::default());
let message = Message::new(
Expand Down Expand Up @@ -7327,30 +7367,15 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
assert_eq!(*elf.get(i).unwrap(), *byte);
}

let loaded_program = bank.load_program(&program_keypair.pubkey(), false, None);
// Advance the bank so that the program becomes effective
goto_end_of_slot(bank.clone());
let bank = bank_client
.advance_slot(1, bank_forks.as_ref(), &mint_keypair.pubkey())
.unwrap();

// Invoke deployed program
mock_process_instruction(
&bpf_loader_upgradeable::id(),
vec![0, 1],
&[],
vec![
(programdata_address, post_programdata_account),
(program_keypair.pubkey(), post_program_account),
],
Vec::new(),
Ok(()),
solana_bpf_loader_program::Entrypoint::vm,
|invoke_context| {
invoke_context
.programs_modified_by_tx
.set_slot_for_tests(bank.slot() + DELAY_VISIBILITY_SLOT_OFFSET);
invoke_context
.programs_modified_by_tx
.replenish(program_keypair.pubkey(), loaded_program.clone());
},
|_invoke_context| {},
);
// Invoke the deployed program
let transaction = Transaction::new(&[&binding], invocation_message, bank.last_blockhash());
assert!(bank.process_transaction(&transaction).is_ok());

// Test initialized program account
bank.clear_signatures();
Expand Down

0 comments on commit f2ea237

Please sign in to comment.