Skip to content

Commit

Permalink
SVM: Refactor program match criteria (anza-xyz#1784)
Browse files Browse the repository at this point in the history 
* SVM: hoist `program_modification_slot` up from bank

* SVM: add `check_program_modification_slot` to processing config

* SVM: hoist `program_match_criteria` up from bank

* SVM: drop `get_program_match_critera` from callbacks
  • Loading branch information
@buffalojoec
buffalojoec authored Jun 21, 2024
1 parent 114d94a commit 0f956c8
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 157 deletions.
1 change: 1 addition & 0 deletions core/src/banking_stage/consumer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ impl Consumer {
&mut execute_and_commit_timings.execute_timings,
TransactionProcessingConfig {
account_overrides: None,
check_program_modification_slot: bank.check_program_modification_slot(),
compute_budget: bank.compute_budget(),
log_messages_bytes_limit: self.log_messages_bytes_limit,
limit_to_load_programs: true,
Expand Down
64 changes: 10 additions & 54 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,15 @@ use {
solana_perf::perf_libs,
solana_program_runtime::{
invoke_context::BuiltinFunctionWithContext,
loaded_programs::{
ProgramCacheEntry, ProgramCacheEntryOwner, ProgramCacheEntryType,
ProgramCacheMatchCriteria,
},
loaded_programs::{ProgramCacheEntry, ProgramCacheEntryOwner, ProgramCacheEntryType},
timings::{ExecuteTimingType, ExecuteTimings},
},
solana_sdk::{
account::{
create_account_shared_data_with_fields as create_account, from_account, Account,
AccountSharedData, InheritableAccountFields, ReadableAccount, WritableAccount,
},
account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
bpf_loader_upgradeable,
clock::{
BankId, Epoch, Slot, SlotCount, SlotIndex, UnixTimestamp, DEFAULT_HASHES_PER_TICK,
DEFAULT_TICKS_PER_SECOND, INITIAL_RENT_EPOCH, MAX_PROCESSING_AGE,
Expand All @@ -133,7 +129,6 @@ use {
incinerator,
inflation::Inflation,
inner_instruction::InnerInstructions,
loader_v4,
message::{AccountKeys, SanitizedMessage},
native_loader,
native_token::LAMPORTS_PER_SOL,
Expand Down Expand Up @@ -3407,6 +3402,7 @@ impl Bank {
&mut timings,
TransactionProcessingConfig {
account_overrides: Some(&account_overrides),
check_program_modification_slot: self.check_program_modification_slot,
compute_budget: self.compute_budget(),
log_messages_bytes_limit: None,
limit_to_load_programs: true,
Expand Down Expand Up @@ -4840,6 +4836,7 @@ impl Bank {
timings,
TransactionProcessingConfig {
account_overrides: None,
check_program_modification_slot: self.check_program_modification_slot,
compute_budget: self.compute_budget(),
log_messages_bytes_limit,
limit_to_load_programs: false,
Expand Down Expand Up @@ -6751,8 +6748,12 @@ impl Bank {
false
}

pub fn check_program_modification_slot(&mut self) {
self.check_program_modification_slot = true;
pub fn check_program_modification_slot(&self) -> bool {
self.check_program_modification_slot
}

pub fn set_check_program_modification_slot(&mut self, check: bool) {
self.check_program_modification_slot = check;
}

pub fn fee_structure(&self) -> &FeeStructure {
Expand All @@ -6767,40 +6768,6 @@ impl Bank {
self.transaction_processor
.add_builtin(self, program_id, name, builtin)
}

/// Find the slot in which the program was most recently modified.
/// Returns slot 0 for programs deployed with v1/v2 loaders, since programs deployed
/// with those loaders do not retain deployment slot information.
/// Returns an error if the program's account state can not be found or parsed.
fn program_modification_slot(&self, pubkey: &Pubkey) -> transaction::Result<Slot> {
let program = self
.get_account(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(&programdata_address)
.ok_or(TransactionError::ProgramAccountNotFound)?;
if let Ok(UpgradeableLoaderState::ProgramData {
slot,
upgrade_authority_address: _,
}) = programdata.state()
{
return Ok(slot);
}
}
Err(TransactionError::ProgramAccountNotFound)
} else if loader_v4::check_id(program.owner()) {
let state = solana_loader_v4_program::get_state(program.data())
.map_err(|_| TransactionError::ProgramAccountNotFound)?;
Ok(state.slot)
} else {
Ok(0)
}
}
}

impl TransactionProcessingCallback for Bank {
Expand All @@ -6820,17 +6787,6 @@ impl TransactionProcessingCallback for Bank {
.map(|(acc, _)| acc)
}

fn get_program_match_criteria(&self, program: &Pubkey) -> ProgramCacheMatchCriteria {
if self.check_program_modification_slot {
self.program_modification_slot(program)
.map_or(ProgramCacheMatchCriteria::Tombstone, |slot| {
ProgramCacheMatchCriteria::DeployedOnOrAfterSlot(slot)
})
} else {
ProgramCacheMatchCriteria::NoCriteria
}
}

// NOTE: must hold idempotent for the same set of arguments
/// Add a builtin program account
fn add_builtin_account(&self, name: &str, program_id: &Pubkey) {
Expand Down
86 changes: 0 additions & 86 deletions runtime/src/bank/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ use {
incinerator,
instruction::{AccountMeta, CompiledInstruction, Instruction, InstructionError},
loader_upgradeable_instruction::UpgradeableLoaderInstruction,
loader_v4::{LoaderV4State, LoaderV4Status},
message::{Message, MessageHeader, SanitizedMessage},
native_loader,
native_token::{sol_to_lamports, LAMPORTS_PER_SOL},
Expand Down Expand Up @@ -13057,91 +13056,6 @@ fn test_deploy_last_epoch_slot() {
assert_eq!(result_with_feature_enabled, Ok(()));
}

#[test]
fn test_program_modification_slot_account_not_found() {
let genesis_config = GenesisConfig::default();
let bank = Bank::new_for_tests(&genesis_config);
let key = Pubkey::new_unique();

let result = bank.program_modification_slot(&key);
assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound));

let mut account_data = AccountSharedData::new(100, 100, &bpf_loader_upgradeable::id());
bank.store_account(&key, &account_data);

let result = bank.program_modification_slot(&key);
assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound));

let state = UpgradeableLoaderState::Program {
programdata_address: Pubkey::new_unique(),
};
account_data.set_data(bincode::serialize(&state).unwrap());
bank.store_account(&key, &account_data);

let result = bank.program_modification_slot(&key);
assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound));

account_data.set_owner(loader_v4::id());
bank.store_account(&key, &account_data);

let result = bank.program_modification_slot(&key);
assert_eq!(result.err(), Some(TransactionError::ProgramAccountNotFound));
}

#[test]
fn test_program_modification_slot_success() {
let genesis_config = GenesisConfig::default();
let bank = Bank::new_for_tests(&genesis_config);

let key1 = Pubkey::new_unique();
let key2 = Pubkey::new_unique();

let account_data = AccountSharedData::new_data(
100,
&UpgradeableLoaderState::Program {
programdata_address: key2,
},
&bpf_loader_upgradeable::id(),
)
.unwrap();
bank.store_account(&key1, &account_data);

let account_data = AccountSharedData::new_data(
100,
&UpgradeableLoaderState::ProgramData {
slot: 77,
upgrade_authority_address: None,
},
&bpf_loader_upgradeable::id(),
)
.unwrap();
bank.store_account(&key2, &account_data);

let result = bank.program_modification_slot(&key1);
assert_eq!(result.unwrap(), 77);

let state = LoaderV4State {
slot: 58,
authority_address: Pubkey::new_unique(),
status: LoaderV4Status::Deployed,
};
let encoded = unsafe {
std::mem::transmute::<&LoaderV4State, &[u8; LoaderV4State::program_data_offset()]>(&state)
};
let mut account_data = AccountSharedData::new(100, encoded.len(), &loader_v4::id());
account_data.set_data(encoded.to_vec());
bank.store_account(&key1, &account_data);

let result = bank.program_modification_slot(&key1);
assert_eq!(result.unwrap(), 58);

account_data.set_owner(Pubkey::new_unique());
bank.store_account(&key2, &account_data);

let result = bank.program_modification_slot(&key2);
assert_eq!(result.unwrap(), 0);
}

#[test]
fn test_blockhash_last_valid_block_height() {
let genesis_config = GenesisConfig::default();
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/bank_forks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ impl BankForks {

pub fn insert(&mut self, mut bank: Bank) -> BankWithScheduler {
if self.root.load(Ordering::Relaxed) < self.highest_slot_at_startup {
bank.check_program_modification_slot();
bank.set_check_program_modification_slot(true);
}

let bank = Arc::new(bank);
Expand Down
Loading

0 comments on commit 0f956c8

Please sign in to comment.