From b162f7b3f521ac13855f992a4827a8ebf8b5be2a Mon Sep 17 00:00:00 2001 From: Sean Young Date: Mon, 8 Jan 2024 11:11:26 +0000 Subject: [PATCH] don't load executable accounts --- ledger-tool/src/program.rs | 3 + program-runtime/src/loaded_programs.rs | 44 ++- programs/bpf_loader/src/lib.rs | 29 +- programs/loader-v4/src/lib.rs | 10 +- programs/sbf/tests/programs.rs | 1 + runtime/src/bank.rs | 2 +- runtime/src/bank/tests.rs | 5 +- sdk/src/feature_set.rs | 10 + svm/src/account_loader.rs | 483 +++++++++++++++++++++---- svm/src/transaction_processor.rs | 94 +++-- 10 files changed, 570 insertions(+), 111 deletions(-) diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index af50d59bca0255..12cd8ee44602b8 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -25,6 +25,7 @@ use { account_utils::StateMut, bpf_loader_upgradeable::{self, UpgradeableLoaderState}, pubkey::Pubkey, + rent_collector::RENT_EXEMPT_RENT_EPOCH, slot_history::Slot, transaction_context::{IndexOfAccount, InstructionAccount}, }, @@ -337,6 +338,8 @@ fn load_program<'a>( &contents, &loader_key, account_size, + RENT_EXEMPT_RENT_EPOCH, + 1, slot, Arc::new(program_runtime_environment), false, diff --git a/program-runtime/src/loaded_programs.rs b/program-runtime/src/loaded_programs.rs index f6163d63cd738c..d3d9c572c9a34a 100644 --- a/program-runtime/src/loaded_programs.rs +++ b/program-runtime/src/loaded_programs.rs @@ -18,6 +18,7 @@ use { clock::{Epoch, Slot}, loader_v4, pubkey::Pubkey, + rent_collector::RENT_EXEMPT_RENT_EPOCH, saturating_add_assign, }, std::{ @@ -133,6 +134,10 @@ pub struct LoadedProgram { pub program: LoadedProgramType, /// Size of account that stores the program and program data pub account_size: usize, + /// lamoports for the program account + pub lamports: u64, + /// rent epoch for the program account + pub rent_epoch: u64, /// Slot in which the program was (re)deployed pub deployment_slot: Slot, /// Slot in which this entry will become active (can be in the future) @@ -282,6 +287,8 @@ impl LoadedProgram { effective_slot: Slot, elf_bytes: &[u8], account_size: usize, + rent_epoch: u64, + lamports: u64, metrics: &mut LoadProgramMetrics, ) -> Result> { Self::new_internal( @@ -291,6 +298,8 @@ impl LoadedProgram { effective_slot, elf_bytes, account_size, + rent_epoch, + lamports, metrics, false, /* reloading */ ) @@ -311,6 +320,8 @@ impl LoadedProgram { effective_slot: Slot, elf_bytes: &[u8], account_size: usize, + rent_epoch: u64, + lamports: u64, metrics: &mut LoadProgramMetrics, ) -> Result> { Self::new_internal( @@ -320,11 +331,14 @@ impl LoadedProgram { effective_slot, elf_bytes, account_size, + rent_epoch, + lamports, metrics, true, /* reloading */ ) } + #[allow(clippy::too_many_arguments)] fn new_internal( loader_key: &Pubkey, program_runtime_environment: Arc>>, @@ -332,6 +346,8 @@ impl LoadedProgram { effective_slot: Slot, elf_bytes: &[u8], account_size: usize, + rent_epoch: u64, + lamports: u64, metrics: &mut LoadProgramMetrics, reloading: bool, ) -> Result> { @@ -371,6 +387,8 @@ impl LoadedProgram { Ok(Self { deployment_slot, account_size, + rent_epoch, + lamports, effective_slot, tx_usage_counter: AtomicU64::new(0), program, @@ -402,6 +420,8 @@ impl LoadedProgram { tx_usage_counter: AtomicU64::new(self.tx_usage_counter.load(Ordering::Relaxed)), ix_usage_counter: AtomicU64::new(self.ix_usage_counter.load(Ordering::Relaxed)), latest_access_slot: AtomicU64::new(self.latest_access_slot.load(Ordering::Relaxed)), + rent_epoch: self.rent_epoch, + lamports: self.lamports, }) } @@ -423,6 +443,8 @@ impl LoadedProgram { program: LoadedProgramType::Builtin(BuiltinProgram::new_builtin(function_registry)), ix_usage_counter: AtomicU64::new(0), latest_access_slot: AtomicU64::new(0), + lamports: 0, + rent_epoch: RENT_EXEMPT_RENT_EPOCH, } } @@ -435,6 +457,8 @@ impl LoadedProgram { tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::new(0), + lamports: 0, + rent_epoch: 0, }; debug_assert!(tombstone.is_tombstone()); tombstone @@ -1164,7 +1188,7 @@ mod tests { assert_matches::assert_matches, percentage::Percentage, solana_rbpf::program::BuiltinProgram, - solana_sdk::{clock::Slot, pubkey::Pubkey}, + solana_sdk::{clock::Slot, pubkey::Pubkey, rent_collector::RENT_EXEMPT_RENT_EPOCH}, std::{ ops::ControlFlow, sync::{ @@ -1204,6 +1228,8 @@ mod tests { tx_usage_counter: usage_counter, ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::new(deployment_slot), + rent_epoch: 0, + lamports: 0, }) } @@ -1216,6 +1242,8 @@ mod tests { tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::default(), + rent_epoch: RENT_EXEMPT_RENT_EPOCH, + lamports: 0, }) } @@ -1246,6 +1274,8 @@ mod tests { tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::default(), + rent_epoch: 0, + lamports: 0, } .to_unloaded() .expect("Failed to unload the program"), @@ -1668,6 +1698,8 @@ mod tests { Arc::new(LoadedProgram { program: old, account_size: 0, + lamports: 0, + rent_epoch: 0, deployment_slot: 10, effective_slot: 11, tx_usage_counter: AtomicU64::default(), @@ -1680,6 +1712,8 @@ mod tests { Arc::new(LoadedProgram { program: new, account_size: 0, + lamports: 0, + rent_epoch: 0, deployment_slot: 10, effective_slot: 11, tx_usage_counter: AtomicU64::default(), @@ -1706,6 +1740,8 @@ mod tests { program: old, account_size: 0, deployment_slot: 10, + lamports: 0, + rent_epoch: 0, effective_slot: 11, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), @@ -1717,6 +1753,8 @@ mod tests { Arc::new(LoadedProgram { program: new, account_size: 0, + lamports: 0, + rent_epoch: 0, deployment_slot: 10, effective_slot: 11, tx_usage_counter: AtomicU64::default(), @@ -1876,6 +1914,8 @@ mod tests { tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::default(), + rent_epoch: RENT_EXEMPT_RENT_EPOCH, + lamports: 0, }); cache.assign_program(program1, updated_program.clone()); @@ -2411,6 +2451,8 @@ mod tests { account_size: 0, deployment_slot: 0, effective_slot: 0, + lamports: 0, + rent_epoch: 0, tx_usage_counter: AtomicU64::default(), ix_usage_counter: AtomicU64::default(), latest_access_slot: AtomicU64::default(), diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index a9c34fbabfc6f6..f4ca93f5b740d0 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -42,6 +42,7 @@ use { loader_v4, native_loader, program_utils::limited_deserialize, pubkey::Pubkey, + rent_collector::RENT_EXEMPT_RENT_EPOCH, saturating_add_assign, system_instruction::{self, MAX_PERMITTED_DATA_LENGTH}, transaction_context::{IndexOfAccount, InstructionContext, TransactionContext}, @@ -66,6 +67,8 @@ pub fn load_program_from_bytes( programdata: &[u8], loader_key: &Pubkey, account_size: usize, + rent_epoch: u64, + lamports: u64, deployment_slot: Slot, program_runtime_environment: Arc>>, reloading: bool, @@ -81,6 +84,8 @@ pub fn load_program_from_bytes( effective_slot, programdata, account_size, + rent_epoch, + lamports, load_program_metrics, ) } @@ -92,6 +97,8 @@ pub fn load_program_from_bytes( effective_slot, programdata, account_size, + rent_epoch, + lamports, load_program_metrics, ) } @@ -104,7 +111,8 @@ pub fn load_program_from_bytes( macro_rules! deploy_program { ($invoke_context:expr, $program_id:expr, $loader_key:expr, - $account_size:expr, $slot:expr, $drop:expr, $new_programdata:expr $(,)?) => {{ + $account_size:expr, $lamports:expr, + $slot:expr, $drop:expr, $new_programdata:expr $(,)?) => {{ let mut load_program_metrics = LoadProgramMetrics::default(); let mut register_syscalls_time = Measure::start("register_syscalls_time"); let deployment_program_runtime_environment = create_program_runtime_environment_v1( @@ -143,6 +151,8 @@ macro_rules! deploy_program { $new_programdata, $loader_key, $account_size, + RENT_EXEMPT_RENT_EPOCH, + $lamports, $slot, $invoke_context.programs_modified_by_tx.environments.program_runtime_v1.clone(), true, @@ -527,7 +537,8 @@ fn process_loader_upgradeable_instruction( ic_logger_msg!(log_collector, "Program account too small"); return Err(InstructionError::AccountDataTooSmall); } - if program.get_lamports() < rent.minimum_balance(program.get_data().len()) { + let lamports = program.get_lamports(); + if lamports < rent.minimum_balance(program.get_data().len()) { ic_logger_msg!(log_collector, "Program account not rent-exempt"); return Err(InstructionError::ExecutableAccountNotRentExempt); } @@ -627,6 +638,7 @@ fn process_loader_upgradeable_instruction( new_program_id, &owner_id, UpgradeableLoaderState::size_of_program().saturating_add(programdata_len), + lamports, clock.slot, { drop(buffer); @@ -735,6 +747,7 @@ fn process_loader_upgradeable_instruction( return Err(InstructionError::InvalidAccountData); } let new_program_id = *program.get_key(); + let lamports = program.get_lamports(); drop(program); // Verify Buffer account @@ -823,6 +836,7 @@ fn process_loader_upgradeable_instruction( new_program_id, program_id, UpgradeableLoaderState::size_of_program().saturating_add(programdata_len), + lamports, clock.slot, { drop(buffer); @@ -1188,6 +1202,7 @@ fn process_loader_upgradeable_instruction( return Err(InstructionError::InvalidAccountOwner); } let program_key = *program_account.get_key(); + let lamports = program_account.get_lamports(); match program_account.get_state()? { UpgradeableLoaderState::Program { programdata_address, @@ -1286,6 +1301,7 @@ fn process_loader_upgradeable_instruction( program_key, &program_id, UpgradeableLoaderState::size_of_program().saturating_add(new_len), + lamports, clock_slot, { drop(programdata_account); @@ -1559,6 +1575,8 @@ pub mod test_utils { account.data(), owner, account.data().len(), + account.rent_epoch(), + account.lamports(), 0, program_runtime_environment.clone(), false, @@ -3780,9 +3798,10 @@ mod tests { program_id, &bpf_loader_upgradeable::id(), elf.len(), + 102, 2, {}, - &elf + &elf, ); Ok(()) } @@ -3801,6 +3820,8 @@ mod tests { tx_usage_counter: AtomicU64::new(100), ix_usage_counter: AtomicU64::new(100), latest_access_slot: AtomicU64::new(0), + rent_epoch: 0, + lamports: 0, }; invoke_context .programs_modified_by_tx @@ -3841,6 +3862,8 @@ mod tests { tx_usage_counter: AtomicU64::new(100), ix_usage_counter: AtomicU64::new(100), latest_access_slot: AtomicU64::new(0), + rent_epoch: 0, + lamports: 0, }; invoke_context .programs_modified_by_tx diff --git a/programs/loader-v4/src/lib.rs b/programs/loader-v4/src/lib.rs index 4764b23fe65e50..a1dc364fc48ce9 100644 --- a/programs/loader-v4/src/lib.rs +++ b/programs/loader-v4/src/lib.rs @@ -26,6 +26,7 @@ use { loader_v4_instruction::LoaderV4Instruction, program_utils::limited_deserialize, pubkey::Pubkey, + rent_collector::RENT_EXEMPT_RENT_EPOCH, saturating_add_assign, transaction_context::{BorrowedAccount, InstructionContext}, }, @@ -410,7 +411,7 @@ pub fn process_instruction_deploy( program_id: buffer.get_key().to_string(), ..LoadProgramMetrics::default() }; - let executor = LoadedProgram::new( + let mut executor = LoadedProgram::new( &loader_v4::id(), invoke_context .programs_modified_by_tx @@ -420,7 +421,9 @@ pub fn process_instruction_deploy( deployment_slot, effective_slot, programdata, - buffer.get_data().len(), + program.get_data().len(), + RENT_EXEMPT_RENT_EPOCH, + program.get_lamports(), &mut load_program_metrics, ) .map_err(|err| { @@ -436,6 +439,7 @@ pub fn process_instruction_deploy( source_program.set_data_length(0, &invoke_context.feature_set)?; source_program.checked_sub_lamports(transfer_lamports, &invoke_context.feature_set)?; program.checked_add_lamports(transfer_lamports, &invoke_context.feature_set)?; + executor.lamports = program.get_lamports(); } let state = get_state_mut(program.get_data_mut(&invoke_context.feature_set)?)?; state.slot = current_slot; @@ -661,6 +665,8 @@ mod tests { 0, programdata, account.data().len(), + account.rent_epoch(), + account.lamports(), &mut load_program_metrics, ) { invoke_context.programs_modified_by_tx.set_slot_for_tests(0); diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 22969bc482a28e..75532ca2c5cbc3 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -4435,6 +4435,7 @@ fn test_deny_executable_write() { if !direct_mapping { feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id()); } + feature_set.deactivate(&feature_set::dont_load_executable_accounts::id()); let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests(); let mut bank_client = BankClient::new_shared(bank); let authority_keypair = Keypair::new(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d1a1805d0d3a20..282fc7e7473084 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -7049,7 +7049,7 @@ impl Bank { self.add_builtin( program_id, "mockup".to_string(), - LoadedProgram::new_builtin(self.slot, 0, builtin_function), + LoadedProgram::new_builtin(self.slot, 1, builtin_function), ); } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 29dbdc2e5aeacd..87a4998c548025 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -7151,10 +7151,7 @@ fn test_bank_load_program() { bank.store_account_and_update_capitalization(&programdata_key, &programdata_account); let program = bank.load_program(&key1, false, bank.epoch()); assert_matches!(program.program, LoadedProgramType::LegacyV1(_)); - assert_eq!( - program.account_size, - program_account.data().len() + programdata_account.data().len() - ); + assert_eq!(program.account_size, program_account.data().len()); } #[test] diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index bb7c50f460fd81..2871549fd5db93 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -784,6 +784,14 @@ pub mod deprecate_unused_legacy_vote_plumbing { solana_sdk::declare_id!("6Uf8S75PVh91MYgPQSHnjRAPQq6an5BDv9vomrCwDqLe"); } +pub mod dont_load_executable_accounts { + solana_sdk::declare_id!("SeanGLG9chXD7bUF42nxUkyhhasZ38RauPFSK5txEkE"); +} + +pub mod dont_load_executable_accounts_no_exceptions { + solana_sdk::declare_id!("Seanx5tafZesNtxNeayrDHj5WFyzfCnsDse2ucj6ATz"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -975,6 +983,8 @@ lazy_static! { (enable_chained_merkle_shreds::id(), "Enable chained Merkle shreds #34916"), (remove_rounding_in_fee_calculation::id(), "Removing unwanted rounding in fee calculation #34982"), (deprecate_unused_legacy_vote_plumbing::id(), "Deprecate unused legacy vote tx plumbing"), + (dont_load_executable_accounts::id(), "executable ro accounts should do not need to be loaded"), + (dont_load_executable_accounts_no_exceptions::id(), "executable ro accounts should not be loaded program_id exceptions"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index ee06dd5fbf2198..ce7754f473c507 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -8,7 +8,7 @@ use { log::warn, solana_program_runtime::{ compute_budget_processor::process_compute_budget_instructions, - loaded_programs::LoadedProgramsForTxBatch, + loaded_programs::{LoadedProgram, LoadedProgramsForTxBatch}, }, solana_sdk::{ account::{ @@ -16,8 +16,9 @@ use { ReadableAccount, WritableAccount, }, feature_set::{ - self, include_loaded_accounts_data_size_in_fee_calculation, - remove_rounding_in_fee_calculation, + self, dont_load_executable_accounts, + include_loaded_accounts_data_size_in_fee_calculation, + remove_rounding_in_fee_calculation, FeatureSet, }, fee::FeeStructure, message::SanitizedMessage, @@ -34,7 +35,7 @@ use { transaction_context::{IndexOfAccount, TransactionAccount}, }, solana_system_program::{get_system_account_kind, SystemAccountKind}, - std::{collections::HashMap, num::NonZeroUsize}, + std::{collections::HashMap, num::NonZeroUsize, sync::Arc}, }; // for the load instructions @@ -117,7 +118,7 @@ pub(crate) fn load_accounts( error_counters: &mut TransactionErrorMetrics, fee_structure: &FeeStructure, account_overrides: Option<&AccountOverrides>, - program_accounts: &HashMap, + program_accounts: &HashMap, loaded_programs: &LoadedProgramsForTxBatch, ) -> Vec { let feature_set = callbacks.get_feature_set(); @@ -185,7 +186,7 @@ fn load_transaction_accounts( fee: u64, error_counters: &mut TransactionErrorMetrics, account_overrides: Option<&AccountOverrides>, - program_accounts: &HashMap, + program_accounts: &HashMap, loaded_programs: &LoadedProgramsForTxBatch, ) -> Result { let feature_set = callbacks.get_feature_set(); @@ -226,52 +227,69 @@ fn load_transaction_accounts( account_overrides.and_then(|overrides| overrides.get(key)) { (account_override.data().len(), account_override.clone(), 0) - } else if let Some(program) = (!instruction_account && !message.is_writable(i)) - .then_some(()) - .and_then(|_| loaded_programs.find(key)) - { - // Optimization to skip loading of accounts which are only used as - // programs in top-level instructions and not passed as instruction accounts. - account_shared_data_from_program(key, program_accounts) - .map(|program_account| (program.account_size, program_account, 0))? + } else if feature_set.is_active(&dont_load_executable_accounts::id()) { + if let Some(program) = (!message.is_writable(i)) + .then_some(()) + .and_then(|_| loaded_programs.find(key)) + { + let (program_owner, _count, load) = program_accounts + .get(key) + .ok_or(TransactionError::AccountNotFound)?; + + if !instruction_account || (!load && !program.is_tombstone()) { + let program_account = account_shared_data_from_program( + program, + program_owner, + instruction_account, + ); + + (program_account.data().len(), program_account, 0) + } else { + load_account( + callbacks, + key, + message, + i, + &feature_set, + rent_collector, + &mut account_found, + ) + } + } else { + load_account( + callbacks, + key, + message, + i, + &feature_set, + rent_collector, + &mut account_found, + ) + } } else { - callbacks - .get_account_shared_data(key) - .map(|mut account| { - if message.is_writable(i) { - if !feature_set - .is_active(&feature_set::disable_rent_fees_collection::id()) - { - let rent_due = rent_collector - .collect_from_existing_account(key, &mut account) - .rent_amount; - - (account.data().len(), account, rent_due) - } else { - // When rent fee collection is disabled, we won't collect rent for any account. If there - // are any rent paying accounts, their `rent_epoch` won't change either. However, if the - // account itself is rent-exempted but its `rent_epoch` is not u64::MAX, we will set its - // `rent_epoch` to u64::MAX. In such case, the behavior stays the same as before. - if account.rent_epoch() != RENT_EXEMPT_RENT_EPOCH - && rent_collector.get_rent_due(&account) == RentDue::Exempt - { - account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); - } - (account.data().len(), account, 0) - } - } else { - (account.data().len(), account, 0) - } - }) - .unwrap_or_else(|| { - account_found = false; - let mut default_account = AccountSharedData::default(); - // All new accounts must be rent-exempt (enforced in Bank::execute_loaded_transaction). - // Currently, rent collection sets rent_epoch to u64::MAX, but initializing the account - // with this field already set would allow us to skip rent collection for these accounts. - default_account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); - (default_account.data().len(), default_account, 0) - }) + if let Some(program) = (!instruction_account && !message.is_writable(i)) + .then_some(()) + .and_then(|_| loaded_programs.find(key)) + { + let (program_owner, _count, _load) = program_accounts + .get(key) + .ok_or(TransactionError::AccountNotFound)?; + + let program_account = + account_shared_data_from_program(program, program_owner, false); + + (program_account.data().len(), program_account, 0) + } else { + load_account( + callbacks, + key, + message, + i, + &feature_set, + rent_collector, + &mut account_found, + ) + } }; accumulate_and_check_loaded_account_data_size( &mut accumulated_accounts_data_size, @@ -411,20 +429,25 @@ fn get_requested_loaded_accounts_data_size_limit( } fn account_shared_data_from_program( - key: &Pubkey, - program_accounts: &HashMap, -) -> Result { + program: Arc, + program_owner: &Pubkey, + pad_with_zeros: bool, +) -> AccountSharedData { // It's an executable program account. The program is already loaded in the cache. // So the account data is not needed. Return a dummy AccountSharedData with meta // information. let mut program_account = AccountSharedData::default(); - let (program_owner, _count) = program_accounts - .get(key) - .ok_or(TransactionError::AccountNotFound)?; - program_account.set_owner(**program_owner); + program_account.set_owner(*program_owner); program_account.set_executable(true); - program_account.set_data_from_slice(create_executable_meta(program_owner)); - Ok(program_account) + let mut data = create_executable_meta(program_owner).to_vec(); + if pad_with_zeros { + data.resize(program.account_size, 0); + } + program_account.set_data_from_slice(&data); + program_account.set_rent_epoch(program.rent_epoch); + program_account.set_lamports(program.lamports); + + program_account } /// Accumulate loaded account data size into `accumulated_accounts_data_size`. @@ -458,27 +481,78 @@ fn construct_instructions_account(message: &SanitizedMessage) -> AccountSharedDa }) } +fn load_account( + callbacks: &CB, + key: &Pubkey, + message: &SanitizedMessage, + i: usize, + feature_set: &Arc, + rent_collector: &RentCollector, + account_found: &mut bool, +) -> (usize, AccountSharedData, u64) { + callbacks + .get_account_shared_data(key) + .map(|mut account| { + if message.is_writable(i) { + if !feature_set.is_active(&feature_set::disable_rent_fees_collection::id()) { + let rent_due = rent_collector + .collect_from_existing_account(key, &mut account) + .rent_amount; + + (account.data().len(), account, rent_due) + } else { + // When rent fee collection is disabled, we won't collect rent for any account. If there + // are any rent paying accounts, their `rent_epoch` won't change either. However, if the + // account itself is rent-exempted but its `rent_epoch` is not u64::MAX, we will set its + // `rent_epoch` to u64::MAX. In such case, the behavior stays the same as before. + if account.rent_epoch() != RENT_EXEMPT_RENT_EPOCH + && rent_collector.get_rent_due(&account) == RentDue::Exempt + { + account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); + } + (account.data().len(), account, 0) + } + } else { + (account.data().len(), account, 0) + } + }) + .unwrap_or_else(|| { + *account_found = false; + let mut default_account = AccountSharedData::default(); + // All new accounts must be rent-exempt (enforced in Bank::execute_loaded_transaction). + // Currently, rent collection sets rent_epoch to u64::MAX, but initializing the account + // with this field already set would allow us to skip rent collection for these accounts. + default_account.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); + (default_account.data().len(), default_account, 0) + }) +} + #[cfg(test)] mod tests { use { super::*, crate::{ transaction_account_state_info::TransactionAccountStateInfo, - transaction_processor::TransactionProcessingCallback, + transaction_processor::{TransactionBatchProcessor, TransactionProcessingCallback}, }, nonce::state::Versions as NonceVersions, solana_program_runtime::{ compute_budget::ComputeBudget, compute_budget_processor, - loaded_programs::{LoadedProgram, LoadedProgramsForTxBatch}, + loaded_programs::{ + BlockRelation, ForkGraph, LoadedProgram, LoadedProgramType, + LoadedProgramsForTxBatch, + }, prioritization_fee::{PrioritizationFeeDetails, PrioritizationFeeType}, + solana_rbpf::program::BuiltinProgram, }, solana_sdk::{ account::{AccountSharedData, ReadableAccount, WritableAccount}, bpf_loader_upgradeable, + clock::Slot, compute_budget::ComputeBudgetInstruction, epoch_schedule::EpochSchedule, - feature_set::FeatureSet, + feature_set::{dont_load_executable_accounts_no_exceptions, FeatureSet}, fee::FeeStructure, hash::Hash, instruction::CompiledInstruction, @@ -499,7 +573,13 @@ mod tests { transaction::{Result, SanitizedTransaction, Transaction, TransactionError}, transaction_context::{TransactionAccount, TransactionContext}, }, - std::{borrow::Cow, collections::HashMap, convert::TryFrom, sync::Arc}, + std::{ + borrow::Cow, + collections::HashMap, + convert::TryFrom, + str::FromStr, + sync::{atomic::AtomicU64, Arc}, + }, }; #[derive(Default)] @@ -510,8 +590,16 @@ mod tests { } impl TransactionProcessingCallback for TestCallbacks { - fn account_matches_owners(&self, _account: &Pubkey, _owners: &[Pubkey]) -> Option { - None + fn account_matches_owners(&self, account: &Pubkey, owners: &[Pubkey]) -> Option { + if let Some(data) = self.accounts_map.get(account) { + if data.lamports() == 0 { + None + } else { + owners.iter().position(|entry| data.owner() == entry) + } + } else { + None + } } fn get_account_shared_data(&self, pubkey: &Pubkey) -> Option { @@ -1414,23 +1502,28 @@ mod tests { #[test] fn test_account_shared_data_from_program() { - let key = Keypair::new().pubkey(); - let other_key = Keypair::new().pubkey(); - - let mut accounts: HashMap = HashMap::new(); - - let result = account_shared_data_from_program(&key, &accounts); - assert_eq!(result.err(), Some(TransactionError::AccountNotFound)); + let program = Arc::new(LoadedProgram { + program: LoadedProgramType::Builtin(BuiltinProgram::new_mock()), + account_size: 0, + deployment_slot: 0, + effective_slot: 0, + tx_usage_counter: AtomicU64::default(), + ix_usage_counter: AtomicU64::default(), + latest_access_slot: AtomicU64::default(), + rent_epoch: RENT_EXEMPT_RENT_EPOCH, + lamports: 0, + }); - accounts.insert(key, (&other_key, 32)); + let owner = Keypair::new().pubkey(); - let result = account_shared_data_from_program(&key, &accounts); + let account = account_shared_data_from_program(program, &owner, false); let mut expected = AccountSharedData::default(); - expected.set_owner(other_key); + expected.set_owner(owner); expected.set_executable(true); - expected.set_data_from_slice(create_executable_meta(&other_key)); + expected.set_rent_epoch(RENT_EXEMPT_RENT_EPOCH); + expected.set_data_from_slice(create_executable_meta(&owner)); - assert_eq!(result.unwrap(), expected); + assert_eq!(account, expected); } #[test] @@ -2272,4 +2365,238 @@ mod tests { vec![(Err(TransactionError::InvalidWritableAccount), None)] ); } + + pub struct MockForkGraph {} + + impl ForkGraph for MockForkGraph { + fn relationship(&self, _a: Slot, _b: Slot) -> BlockRelation { + todo!() + } + } + + #[test] + fn test_dont_load_executable_accounts() { + // needy -> program that requires all executables to loaded (IRL programs in the must_load_all_programs_for_instruction list) + // deserving -> regular program that does not require executables to be loaded. + + // if an executable only appears in the message accounts/program_ids then it does not need to be loaded, it will never + // be serialized + + let mut feature_set = FeatureSet::all_enabled(); + feature_set.deactivate(&dont_load_executable_accounts_no_exceptions::id()); + + let mut mock_bank = TestCallbacks { + feature_set: Arc::new(feature_set), + ..Default::default() + }; + let mut loaded_programs = LoadedProgramsForTxBatch::default(); + + let payer_key = Pubkey::new_unique(); + println!("payer:{payer_key}"); + + let mut account_data = AccountSharedData::default(); + account_data.set_lamports(200000); + mock_bank.accounts_map.insert(payer_key, account_data); + + let needy_key = Pubkey::from_str("sspUE1vrh7xRoXxGsg7vR1zde2WdGtJRbyK9uRumBDy").unwrap(); + println!("needy:{needy_key}"); + + let mut account_data = AccountSharedData::default(); + account_data.set_executable(true); + account_data.set_rent_epoch(1105); + account_data.set_lamports(1002); + account_data.set_owner(native_loader::id()); + account_data.set_data_from_slice(b"Iamsoneedy"); + mock_bank.accounts_map.insert(needy_key, account_data); + + let needy_program: LoadedProgram = LoadedProgram { + program: LoadedProgramType::Builtin(BuiltinProgram::new_mock()), + rent_epoch: 105, + account_size: 100, + lamports: 2, + ..Default::default() + }; + + loaded_programs.replenish(needy_key, Arc::new(needy_program)); + + let deserving_key = Pubkey::new_unique(); + println!("deserving:{deserving_key}"); + let mut account_data = AccountSharedData::default(); + account_data.set_executable(true); + account_data.set_rent_epoch(1106); + account_data.set_lamports(1003); + account_data.set_owner(native_loader::id()); + account_data.set_data_from_slice(b"Iamsodeserving"); + mock_bank.accounts_map.insert(deserving_key, account_data); + + let deserving_program = LoadedProgram { + program: LoadedProgramType::Builtin(BuiltinProgram::new_mock()), + rent_epoch: 106, + account_size: 4, + lamports: 3, + ..Default::default() + }; + loaded_programs.replenish(deserving_key, Arc::new(deserving_program)); + + let mut error_counter = TransactionErrorMetrics::default(); + + let mut test = |message: Message, + load_needy: bool, + needy_padding: bool, + load_deserving: bool, + deserving_padding: bool| { + let legacy = LegacyMessage::new(message); + let sanitized_message = SanitizedMessage::Legacy(legacy); + + let sanitized_transaction = [SanitizedTransaction::new_for_tests( + sanitized_message, + vec![Signature::new_unique()], + false, + )]; + let mut lock_results = + [(Ok(()), Some(NoncePartial::default()), Some(20u64)) as TransactionCheckResult]; + + let program_owners = [native_loader::id()]; + + let program_accounts = + TransactionBatchProcessor::::filter_executable_program_accounts( + &mock_bank, + &sanitized_transaction, + &mut lock_results, + &program_owners, + ); + + let results = load_accounts( + &mock_bank, + &sanitized_transaction, + &lock_results, + &mut error_counter, + &FeeStructure::default(), + None, + &program_accounts, + &loaded_programs, + ); + + assert_eq!(results.len(), 1); + + let accounts = &results[0].0.as_ref().unwrap().accounts; + + assert_eq!(accounts.len(), 3); + + assert_eq!(accounts[0].0, payer_key); + assert_eq!(accounts[0].1.rent_epoch(), 0); + assert_eq!(accounts[0].1.lamports(), 200000); + assert_eq!(accounts[0].1.data(), b""); + + if load_needy { + assert_eq!(accounts[1].0, needy_key); + assert_eq!(accounts[1].1.rent_epoch(), 1105); + assert_eq!(accounts[1].1.lamports(), 1002); + assert_eq!(accounts[1].1.data(), b"Iamsoneedy"); + } else { + assert_eq!(accounts[1].0, needy_key); + assert_eq!(accounts[1].1.rent_epoch(), 105); + assert_eq!(accounts[1].1.lamports(), 2); + let mut data = vec![1u8]; + if needy_padding { + data.resize(100, 0); + } + assert_eq!(accounts[1].1.data(), data); + } + + if load_deserving { + assert_eq!(accounts[2].0, deserving_key); + assert_eq!(accounts[2].1.rent_epoch(), 1106); + assert_eq!(accounts[2].1.lamports(), 1003); + assert_eq!(accounts[2].1.data(), b"Iamsodeserving"); + } else { + assert_eq!(accounts[2].0, deserving_key); + assert_eq!(accounts[2].1.rent_epoch(), 106); + assert_eq!(accounts[2].1.lamports(), 3); + let mut data = vec![1u8]; + // The zero-padding happens when the account is an instruction account + if deserving_padding { + data.resize(4, 0); + } + assert_eq!(accounts[2].1.data(), data); + } + }; + + // a transaction that has a needy and deserving key, and the needy is passed + // in as an instruction account + let message = Message { + account_keys: vec![payer_key, needy_key, deserving_key], + header: MessageHeader { + num_readonly_unsigned_accounts: 2, + ..Default::default() + }, + instructions: vec![CompiledInstruction { + program_id_index: 2, + accounts: vec![1], + data: vec![], + }], + recent_blockhash: Hash::default(), + }; + + test(message, true, true, false, false); + + // a program that requires executables to be loaded + // needy requires deserving to be loaded, but needy itself it not passed + let message = Message { + account_keys: vec![payer_key, needy_key, deserving_key], + header: MessageHeader { + num_readonly_unsigned_accounts: 2, + ..Default::default() + }, + instructions: vec![CompiledInstruction { + program_id_index: 1, + accounts: vec![2], + data: vec![], + }], + recent_blockhash: Hash::default(), + }; + + test(message, false, false, true, true); + + // needy does not have any accounts passed in -> nothing should be loaded + let message = Message { + account_keys: vec![payer_key, needy_key, deserving_key], + header: MessageHeader { + num_readonly_unsigned_accounts: 3, + ..Default::default() + }, + instructions: vec![ + CompiledInstruction { + program_id_index: 1, + accounts: vec![], + data: vec![], + }, + CompiledInstruction { + program_id_index: 2, + accounts: vec![], + data: vec![], + }, + ], + recent_blockhash: Hash::default(), + }; + + test(message, false, false, false, false); + + // needy does have any accounts passed in -> nothing should be loaded + let message = Message { + account_keys: vec![payer_key, needy_key, deserving_key], + header: MessageHeader { + num_readonly_unsigned_accounts: 3, + ..Default::default() + }, + instructions: vec![CompiledInstruction { + program_id_index: 1, + accounts: vec![2], + data: vec![], + }], + recent_blockhash: Hash::default(), + }; + + test(message, false, false, true, true); + } } diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index c42566fc9876f9..bdb7fea2e8a7cb 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -32,7 +32,7 @@ use { bpf_loader_upgradeable::{self, UpgradeableLoaderState}, clock::{Epoch, Slot}, epoch_schedule::EpochSchedule, - feature_set::FeatureSet, + feature_set::{dont_load_executable_accounts_no_exceptions, FeatureSet}, fee::FeeStructure, hash::Hash, inner_instruction::{InnerInstruction, InnerInstructionsList}, @@ -55,6 +55,22 @@ use { }, }; +// Mainnet-beta program_ids that need executable accounts to be loaded +solana_sdk::pubkeys!( + must_load_all_programs_for_instruction, + [ + "sspUE1vrh7xRoXxGsg7vR1zde2WdGtJRbyK9uRumBDy", + // Program log: Wrong account. Expected: 11111111111111111111111111111111, Got: EmiU8AQkB2sswTxVB6aCmsAJftoowZGGDXuytm6X65R3 + "sp1V4h2gWorkGhVcazBc22Hfo2f5sd7jcjT4EDPrWFF", + // Program mare3SCyfZkAndpBRBeonETmkCCB3TJTTrz8ZN2dnhP failed: custom program error: 0x3ee + "mare3SCyfZkAndpBRBeonETmkCCB3TJTTrz8ZN2dnhP", + // Program 1idUSy4MGGKyKhvjSnGZ6Zc7Q4eKQcibym4BkEEw9KR failed: custom program error: 0x3ee + "1idUSy4MGGKyKhvjSnGZ6Zc7Q4eKQcibym4BkEEw9KR", + // Program log: AnchorError caused by account: program. Error Code: AdminOnly. Error Number: 6016. Error Message: This is an admin only function. + "RAFFLv4sQoBPqLLqQvSHLRSFNnnoNekAbXfSegbQygF", + ] +); + /// A list of log messages emitted during a transaction pub type TransactionLogMessages = Vec; @@ -218,7 +234,7 @@ impl TransactionBatchProcessor { ); let native_loader = native_loader::id(); for builtin_program in builtin_programs { - program_accounts_map.insert(*builtin_program, (&native_loader, 0)); + program_accounts_map.insert(*builtin_program, (&native_loader, 0, false)); } let programs_loaded_for_tx_batch = Rc::new(RefCell::new(self.replenish_program_cache( @@ -339,23 +355,37 @@ impl TransactionBatchProcessor { /// Returns a hash map of executable program accounts (program accounts that are not writable /// in the given transactions), and their owners, for the transactions with a valid /// blockhash or nonce. - fn filter_executable_program_accounts<'a, CB: TransactionProcessingCallback>( + pub fn filter_executable_program_accounts<'a, CB: TransactionProcessingCallback>( callbacks: &CB, txs: &[SanitizedTransaction], lock_results: &mut [TransactionCheckResult], program_owners: &'a [Pubkey], - ) -> HashMap { - let mut result: HashMap = HashMap::new(); + ) -> HashMap { + let check_for_exceptions = !callbacks + .get_feature_set() + .is_active(&dont_load_executable_accounts_no_exceptions::id()); + + let mut result: HashMap = HashMap::new(); lock_results.iter_mut().zip(txs).for_each(|etx| { if let ((Ok(()), _nonce, lamports_per_signature), tx) = etx { if lamports_per_signature.is_some() { + // if any of the programs is in the exception list, then all programs for this transaction + // should be loaded + let load_all_programs = check_for_exceptions + && tx + .message() + .account_keys() + .iter() + .any(|key| must_load_all_programs_for_instruction().contains(key)); + tx.message() .account_keys() .iter() .for_each(|key| match result.entry(*key) { Entry::Occupied(mut entry) => { - let (_, count) = entry.get_mut(); + let (_, count, load) = entry.get_mut(); saturating_add_assign!(*count, 1); + *load |= load_all_programs; } Entry::Vacant(entry) => { if let Some(index) = @@ -363,7 +393,7 @@ impl TransactionBatchProcessor { { program_owners .get(index) - .map(|owner| entry.insert((owner, 1))); + .map(|owner| entry.insert((owner, 1, load_all_programs))); } } }); @@ -409,6 +439,8 @@ impl TransactionBatchProcessor { program_account.data(), program_account.owner(), program_account.data().len(), + program_account.rent_epoch(), + program_account.lamports(), 0, environments.program_runtime_v1.clone(), reload, @@ -429,10 +461,9 @@ impl TransactionBatchProcessor { &mut load_program_metrics, programdata, program_account.owner(), - program_account - .data() - .len() - .saturating_add(programdata_account.data().len()), + program_account.data().len(), + program_account.rent_epoch(), + program_account.lamports(), slot, environments.program_runtime_v1.clone(), reload, @@ -451,6 +482,8 @@ impl TransactionBatchProcessor { elf_bytes, &loader_v4::id(), program_account.data().len(), + program_account.rent_epoch(), + program_account.lamports(), slot, environments.program_runtime_v2.clone(), reload, @@ -487,14 +520,14 @@ impl TransactionBatchProcessor { fn replenish_program_cache( &self, callback: &CB, - program_accounts_map: &HashMap, + program_accounts_map: &HashMap, limit_to_load_programs: bool, ) -> LoadedProgramsForTxBatch { let mut missing_programs: Vec<(Pubkey, (LoadedProgramMatchCriteria, u64))> = if self.check_program_modification_slot { program_accounts_map .iter() - .map(|(pubkey, (_, count))| { + .map(|(pubkey, (_, count, _))| { ( *pubkey, ( @@ -510,7 +543,7 @@ impl TransactionBatchProcessor { } else { program_accounts_map .iter() - .map(|(pubkey, (_, count))| { + .map(|(pubkey, (_, count, _))| { (*pubkey, (LoadedProgramMatchCriteria::NoCriteria, *count)) }) .collect() @@ -802,6 +835,8 @@ impl TransactionBatchProcessor { programdata: &[u8], loader_key: &Pubkey, account_size: usize, + rent_epoch: u64, + lamports: u64, deployment_slot: Slot, program_runtime_environment: ProgramRuntimeEnvironment, reloading: bool, @@ -816,6 +851,8 @@ impl TransactionBatchProcessor { deployment_slot.saturating_add(DELAY_VISIBILITY_SLOT_OFFSET), programdata, account_size, + rent_epoch, + lamports, load_program_metrics, ) } @@ -827,6 +864,8 @@ impl TransactionBatchProcessor { deployment_slot.saturating_add(DELAY_VISIBILITY_SLOT_OFFSET), programdata, account_size, + rent_epoch, + lamports, load_program_metrics, ) } @@ -952,6 +991,7 @@ mod tests { account::WritableAccount, bpf_loader, message::{LegacyMessage, Message, MessageHeader}, + rent_collector::RENT_EXEMPT_RENT_EPOCH, rent_debits::RentDebits, signature::{Keypair, Signature}, sysvar::rent::Rent, @@ -1232,6 +1272,8 @@ mod tests { &buffer, &loader, size, + RENT_EXEMPT_RENT_EPOCH, + 0, slot, environment.clone(), false, @@ -1244,6 +1286,8 @@ mod tests { &buffer, &loader, size, + RENT_EXEMPT_RENT_EPOCH, + 0, slot, environment, true, @@ -1340,6 +1384,8 @@ mod tests { account_data.data(), account_data.owner(), account_data.data().len(), + RENT_EXEMPT_RENT_EPOCH, + 0, 0, environments.program_runtime_v1.clone(), false, @@ -1427,6 +1473,8 @@ mod tests { account_data.data(), account_data.owner(), account_data.data().len(), + RENT_EXEMPT_RENT_EPOCH, + 0, 0, environments.program_runtime_v1.clone(), false, @@ -1505,6 +1553,8 @@ mod tests { account_data.data(), account_data.owner(), account_data.data().len(), + RENT_EXEMPT_RENT_EPOCH, + 0, 0, environments.program_runtime_v1.clone(), false, @@ -1832,10 +1882,10 @@ mod tests { account_data.set_owner(bpf_loader::id()); mock_bank.account_shared_data.insert(key2, account_data); - let mut account_maps: HashMap = HashMap::new(); - account_maps.insert(key1, (&owner, 2)); + let mut account_maps: HashMap = HashMap::new(); + account_maps.insert(key1, (&owner, 2, false)); - account_maps.insert(key2, (&owner, 4)); + account_maps.insert(key2, (&owner, 4, false)); let result = batch_processor.replenish_program_cache(&mock_bank, &account_maps, false); let program1 = result.find(&key1).unwrap(); @@ -1947,8 +1997,8 @@ mod tests { (Err(TransactionError::BlockhashNotFound), None, None) ); assert_eq!(result.len(), 2); - assert_eq!(result[&key1], (&owner1, 2)); - assert_eq!(result[&key2], (&owner2, 1)); + assert_eq!(result[&key1], (&owner1, 2, false)); + assert_eq!(result[&key2], (&owner2, 1, false)); } #[test] @@ -2034,13 +2084,13 @@ mod tests { programs .get(&account3_pubkey) .expect("failed to find the program account"), - &(&program1_pubkey, 2) + &(&program1_pubkey, 2, false) ); assert_eq!( programs .get(&account4_pubkey) .expect("failed to find the program account"), - &(&program2_pubkey, 1) + &(&program2_pubkey, 1, false) ); } @@ -2129,7 +2179,7 @@ mod tests { programs .get(&account3_pubkey) .expect("failed to find the program account"), - &(&program1_pubkey, 1) + &(&program1_pubkey, 1, false) ); assert_eq!(lock_results[1].0, Err(TransactionError::BlockhashNotFound)); }