Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SVM: Refactor program match criteria #1784

Merged
merged 4 commits into from
Jun 21, 2024
Merged

SVM: Refactor program match criteria #1784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9f6d8f6
SVM: hoist `program_modification_slot` up from bank
buffalojoec Jun 18, 2024
653e3a9
SVM: add `check_program_modification_slot` to processing config
buffalojoec Jun 18, 2024
334b73a
SVM: hoist `program_match_criteria` up from bank
buffalojoec Jun 18, 2024
7a8d3a9
SVM: drop `get_program_match_critera` from callbacks
buffalojoec Jun 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/src/banking_stage/consumer.rs
View file
Open in desktop
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
Open in desktop
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 @@ -4844,6 +4840,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 @@ -6755,8 +6752,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 @@ -6771,40 +6772,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 @@ -6824,17 +6791,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
Open in desktop
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
Open in desktop
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