From 16a91ee159da4f6c4529ec572005365cb819ffa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Mon, 8 Jul 2024 16:31:38 +0200 Subject: [PATCH 1/5] Adds Serializer::infer_is_executable() --- ledger-tool/src/program.rs | 1 + program-test/src/lib.rs | 1 + programs/bpf_loader/benches/serialization.rs | 64 +++++++++++++++++--- programs/bpf_loader/src/lib.rs | 1 + programs/bpf_loader/src/serialization.rs | 59 +++++++++++++++--- programs/sbf/benches/bpf_loader.rs | 2 + 6 files changed, 111 insertions(+), 17 deletions(-) diff --git a/ledger-tool/src/program.rs b/ledger-tool/src/program.rs index 463d017b17dbed..a445b46db9fdab 100644 --- a/ledger-tool/src/program.rs +++ b/ledger-tool/src/program.rs @@ -544,6 +544,7 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) { .get_current_instruction_context() .unwrap(), true, // copy_account_data + Some(invoke_context.program_cache_for_tx_batch), ) .unwrap(); diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 7fd19cb963cd19..592dc66ad2fa80 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -136,6 +136,7 @@ pub fn invoke_builtin_function( .transaction_context .get_current_instruction_context()?, true, // copy_account_data // There is no VM so direct mapping can not be implemented here + Some(invoke_context.program_cache_for_tx_batch), )?; // Deserialize data back into instruction params diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index 5d3c55a165e399..2088200d8b6f1a 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -4,6 +4,7 @@ extern crate test; use { solana_bpf_loader_program::serialization::serialize_parameters, + solana_program_runtime::loaded_programs::{ProgramCacheForTxBatch, ProgramRuntimeEnvironments}, solana_sdk::{ account::{Account, AccountSharedData}, bpf_loader, bpf_loader_deprecated, @@ -125,8 +126,16 @@ fn bench_serialize_unaligned(bencher: &mut Bencher) { let instruction_context = transaction_context .get_current_instruction_context() .unwrap(); + let program_cache_for_tx_batch = + ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, false).unwrap(); + let _ = serialize_parameters( + &transaction_context, + instruction_context, + false, + Some(&program_cache_for_tx_batch), + ) + .unwrap(); }); } @@ -136,8 +145,16 @@ fn bench_serialize_unaligned_copy_account_data(bencher: &mut Bencher) { let instruction_context = transaction_context .get_current_instruction_context() .unwrap(); + let program_cache_for_tx_batch = + ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, true).unwrap(); + let _ = serialize_parameters( + &transaction_context, + instruction_context, + true, + Some(&program_cache_for_tx_batch), + ) + .unwrap(); }); } @@ -147,9 +164,16 @@ fn bench_serialize_aligned(bencher: &mut Bencher) { let instruction_context = transaction_context .get_current_instruction_context() .unwrap(); - + let program_cache_for_tx_batch = + ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, false).unwrap(); + let _ = serialize_parameters( + &transaction_context, + instruction_context, + false, + Some(&program_cache_for_tx_batch), + ) + .unwrap(); }); } @@ -159,9 +183,16 @@ fn bench_serialize_aligned_copy_account_data(bencher: &mut Bencher) { let instruction_context = transaction_context .get_current_instruction_context() .unwrap(); - + let program_cache_for_tx_batch = + ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, true).unwrap(); + let _ = serialize_parameters( + &transaction_context, + instruction_context, + true, + Some(&program_cache_for_tx_batch), + ) + .unwrap(); }); } @@ -171,8 +202,16 @@ fn bench_serialize_unaligned_max_accounts(bencher: &mut Bencher) { let instruction_context = transaction_context .get_current_instruction_context() .unwrap(); + let program_cache_for_tx_batch = + ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, false).unwrap(); + let _ = serialize_parameters( + &transaction_context, + instruction_context, + false, + Some(&program_cache_for_tx_batch), + ) + .unwrap(); }); } @@ -182,8 +221,15 @@ fn bench_serialize_aligned_max_accounts(bencher: &mut Bencher) { let instruction_context = transaction_context .get_current_instruction_context() .unwrap(); - + let program_cache_for_tx_batch = + ProgramCacheForTxBatch::new(0, ProgramRuntimeEnvironments::default(), None, 0); bencher.iter(|| { - let _ = serialize_parameters(&transaction_context, instruction_context, false).unwrap(); + let _ = serialize_parameters( + &transaction_context, + instruction_context, + false, + Some(&program_cache_for_tx_batch), + ) + .unwrap(); }); } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 494477e8a04363..d8a0757f7c08c1 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -1367,6 +1367,7 @@ fn execute<'a, 'b: 'a>( invoke_context.transaction_context, instruction_context, !direct_mapping, + Some(invoke_context.program_cache_for_tx_batch), )?; serialize_time.stop(); diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs index 2cb2f20b162280..bdfd98fdc9d9d9 100644 --- a/programs/bpf_loader/src/serialization.rs +++ b/programs/bpf_loader/src/serialization.rs @@ -2,7 +2,10 @@ use { byteorder::{ByteOrder, LittleEndian}, - solana_program_runtime::invoke_context::SerializedAccountMetadata, + solana_program_runtime::{ + invoke_context::SerializedAccountMetadata, + loaded_programs::{ProgramCacheEntryType, ProgramCacheForTxBatch}, + }, solana_rbpf::{ aligned_memory::{AlignedMemory, Pod}, ebpf::{HOST_ALIGN, MM_INPUT_START}, @@ -31,17 +34,24 @@ enum SerializeAccount<'a> { Duplicate(IndexOfAccount), } -struct Serializer { +struct Serializer<'a> { pub buffer: AlignedMemory, regions: Vec, vaddr: u64, region_start: usize, aligned: bool, copy_account_data: bool, + program_cache_for_tx_batch: Option<&'a ProgramCacheForTxBatch>, } -impl Serializer { - fn new(size: usize, start_addr: u64, aligned: bool, copy_account_data: bool) -> Serializer { +impl<'a> Serializer<'a> { + fn new( + size: usize, + start_addr: u64, + aligned: bool, + copy_account_data: bool, + program_cache_for_tx_batch: Option<&'a ProgramCacheForTxBatch>, + ) -> Serializer { Serializer { buffer: AlignedMemory::with_capacity(size), regions: Vec::new(), @@ -49,6 +59,7 @@ impl Serializer { vaddr: start_addr, aligned, copy_account_data, + program_cache_for_tx_batch, } } @@ -174,6 +185,18 @@ impl Serializer { (self.buffer, self.regions) } + /// Replaces `account.is_executable()` + fn infer_is_executable(&self, account: &BorrowedAccount<'_>) -> bool { + if let Some(program_cache_for_tx_batch) = self.program_cache_for_tx_batch { + program_cache_for_tx_batch + .find(account.get_key()) + .map(|entry| !matches!(entry.program, ProgramCacheEntryType::Closed)) + .unwrap_or(false) + } else { + account.is_executable() + } + } + fn debug_assert_alignment(&self) { debug_assert!( !self.aligned @@ -192,6 +215,7 @@ pub fn serialize_parameters( transaction_context: &TransactionContext, instruction_context: &InstructionContext, copy_account_data: bool, + program_cache_for_tx_batch: Option<&ProgramCacheForTxBatch>, ) -> Result< ( AlignedMemory, @@ -240,6 +264,7 @@ pub fn serialize_parameters( instruction_context.get_instruction_data(), &program_id, copy_account_data, + program_cache_for_tx_batch, ) } else { serialize_parameters_aligned( @@ -247,6 +272,7 @@ pub fn serialize_parameters( instruction_context.get_instruction_data(), &program_id, copy_account_data, + program_cache_for_tx_batch, ) } } @@ -287,6 +313,7 @@ fn serialize_parameters_unaligned( instruction_data: &[u8], program_id: &Pubkey, copy_account_data: bool, + program_cache_for_tx_batch: Option<&ProgramCacheForTxBatch>, ) -> Result< ( AlignedMemory, @@ -320,7 +347,13 @@ fn serialize_parameters_unaligned( + instruction_data.len() // instruction data + size_of::(); // program id - let mut s = Serializer::new(size, MM_INPUT_START, false, copy_account_data); + let mut s = Serializer::new( + size, + MM_INPUT_START, + false, + copy_account_data, + program_cache_for_tx_batch, + ); let mut accounts_metadata: Vec = Vec::with_capacity(accounts.len()); s.write::((accounts.len() as u64).to_le()); @@ -339,7 +372,7 @@ fn serialize_parameters_unaligned( s.write::((account.get_data().len() as u64).to_le()); let vm_data_addr = s.write_account(&mut account)?; let vm_owner_addr = s.write_all(account.get_owner().as_ref()); - s.write::(account.is_executable() as u8); + s.write::(s.infer_is_executable(&account) as u8); s.write::((account.get_rent_epoch()).to_le()); accounts_metadata.push(SerializedAccountMetadata { original_data_len: account.get_data().len(), @@ -418,6 +451,7 @@ fn serialize_parameters_aligned( instruction_data: &[u8], program_id: &Pubkey, copy_account_data: bool, + program_cache_for_tx_batch: Option<&ProgramCacheForTxBatch>, ) -> Result< ( AlignedMemory, @@ -457,7 +491,13 @@ fn serialize_parameters_aligned( + instruction_data.len() + size_of::(); // program id; - let mut s = Serializer::new(size, MM_INPUT_START, true, copy_account_data); + let mut s = Serializer::new( + size, + MM_INPUT_START, + true, + copy_account_data, + program_cache_for_tx_batch, + ); // Serialize into the buffer s.write::((accounts.len() as u64).to_le()); @@ -467,7 +507,7 @@ fn serialize_parameters_aligned( s.write::(NON_DUP_MARKER); s.write::(borrowed_account.is_signer() as u8); s.write::(borrowed_account.is_writable() as u8); - s.write::(borrowed_account.is_executable() as u8); + s.write::(s.infer_is_executable(&borrowed_account) as u8); s.write_all(&[0u8, 0, 0, 0]); let vm_key_addr = s.write_all(borrowed_account.get_key().as_ref()); let vm_owner_addr = s.write_all(borrowed_account.get_owner().as_ref()); @@ -729,6 +769,7 @@ mod tests { invoke_context.transaction_context, instruction_context, copy_account_data, + Some(invoke_context.program_cache_for_tx_batch), ); assert_eq!( serialization_result.as_ref().err(), @@ -883,6 +924,7 @@ mod tests { invoke_context.transaction_context, instruction_context, copy_account_data, + Some(invoke_context.program_cache_for_tx_batch), ) .unwrap(); @@ -974,6 +1016,7 @@ mod tests { invoke_context.transaction_context, instruction_context, copy_account_data, + Some(invoke_context.program_cache_for_tx_batch), ) .unwrap(); let mut serialized_regions = concat_regions(®ions); diff --git a/programs/sbf/benches/bpf_loader.rs b/programs/sbf/benches/bpf_loader.rs index ab5e950faab874..eaaeff485bbec2 100644 --- a/programs/sbf/benches/bpf_loader.rs +++ b/programs/sbf/benches/bpf_loader.rs @@ -258,6 +258,7 @@ fn bench_create_vm(bencher: &mut Bencher) { .get_current_instruction_context() .unwrap(), !direct_mapping, // copy_account_data, + Some(invoke_context.program_cache_for_tx_batch), ) .unwrap(); @@ -292,6 +293,7 @@ fn bench_instruction_count_tuner(_bencher: &mut Bencher) { .get_current_instruction_context() .unwrap(), !direct_mapping, // copy_account_data + Some(invoke_context.program_cache_for_tx_batch), ) .unwrap(); From b8d3e9d655f3cc0cc83ac7cf4330d451f86eed11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 10 Jul 2024 10:48:01 +0200 Subject: [PATCH 2/5] Adds the feature. --- sdk/src/feature_set.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 4e5100dc3473f8..f5c8765a9b0005 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -841,6 +841,10 @@ pub mod ed25519_precompile_verify_strict { solana_sdk::declare_id!("ed9tNscbWLYBooxWA7FE2B5KHWs8A6sxfY8EzezEcoo"); } +pub mod infer_account_is_executable_flag { + solana_sdk::declare_id!("FfgtauHUWKeXTzjXkua9Px4tNGBFHKZ9WaigM5VbbzFx"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -1046,6 +1050,7 @@ lazy_static! { (verify_retransmitter_signature::id(), "Verify retransmitter signature #1840"), (move_stake_and_move_lamports_ixs::id(), "Enable MoveStake and MoveLamports stake program instructions #1610"), (ed25519_precompile_verify_strict::id(), "Use strict verification in ed25519 precompile SIMD-0152"), + (infer_account_is_executable_flag::id(), "Infer accounts is_executable flag from their owner #2034"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() From a0c1f503a56d6346765566657460b2e3bb6de6ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 10 Jul 2024 10:48:01 +0200 Subject: [PATCH 3/5] Adds the feature gate logic. --- program-runtime/src/invoke_context.rs | 8 ++++-- programs/bpf_loader/src/lib.rs | 38 ++++++++++++++++++------- programs/bpf_loader/src/syscalls/cpi.rs | 18 ++++++++++-- sdk/src/transaction_context.rs | 17 ++++++++++- svm/src/account_loader.rs | 8 ++++-- svm/src/transaction_processor.rs | 7 ++++- 6 files changed, 78 insertions(+), 18 deletions(-) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 0012422cca3abd..b587dd4a9fc979 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -23,7 +23,7 @@ use { bpf_loader_deprecated, clock::Slot, epoch_schedule::EpochSchedule, - feature_set::FeatureSet, + feature_set::{infer_account_is_executable_flag, FeatureSet}, hash::Hash, instruction::{AccountMeta, InstructionError}, native_loader, @@ -434,7 +434,11 @@ impl<'a> InvokeContext<'a> { })?; let borrowed_program_account = instruction_context .try_borrow_instruction_account(self.transaction_context, program_account_index)?; - if !borrowed_program_account.is_executable() { + if !self + .get_feature_set() + .is_active(&infer_account_is_executable_flag::id()) + && !borrowed_program_account.is_executable() + { ic_msg!(self, "Account {} is not executable", callee_program_id); return Err(InstructionError::AccountNotExecutable); } diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index d8a0757f7c08c1..246c6703365b90 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -36,6 +36,7 @@ use { entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, feature_set::{ bpf_account_data_direct_mapping, enable_bpf_loader_set_authority_checked_ix, + infer_account_is_executable_flag, }, instruction::{AccountMeta, InstructionError}, loader_upgradeable_instruction::UpgradeableLoaderInstruction, @@ -430,7 +431,11 @@ pub fn process_instruction_inner( } // Program Invocation - if !program_account.is_executable() { + if !invoke_context + .get_feature_set() + .is_active(&infer_account_is_executable_flag::id()) + && !program_account.is_executable() + { ic_logger_msg!(log_collector, "Program is not executable"); return Err(Box::new(InstructionError::IncorrectProgramId)); } @@ -718,7 +723,11 @@ fn process_loader_upgradeable_instruction( let program = instruction_context.try_borrow_instruction_account(transaction_context, 1)?; - if !program.is_executable() { + if !invoke_context + .get_feature_set() + .is_active(&infer_account_is_executable_flag::id()) + && !program.is_executable() + { ic_logger_msg!(log_collector, "Program account not executable"); return Err(InstructionError::AccountNotExecutable); } @@ -1367,7 +1376,10 @@ fn execute<'a, 'b: 'a>( invoke_context.transaction_context, instruction_context, !direct_mapping, - Some(invoke_context.program_cache_for_tx_batch), + invoke_context + .get_feature_set() + .is_active(&infer_account_is_executable_flag::id()) + .then_some(invoke_context.program_cache_for_tx_batch), )?; serialize_time.stop(); @@ -1458,13 +1470,19 @@ fn execute<'a, 'b: 'a>( instruction_account_index as IndexOfAccount, )?; - error = EbpfError::SyscallError(Box::new(if account.is_executable() { - InstructionError::ExecutableDataModified - } else if account.is_writable() { - InstructionError::ExternalAccountDataModified - } else { - InstructionError::ReadonlyDataModified - })); + error = EbpfError::SyscallError(Box::new( + if !invoke_context + .get_feature_set() + .is_active(&infer_account_is_executable_flag::id()) + && account.is_executable() + { + InstructionError::ExecutableDataModified + } else if account.is_writable() { + InstructionError::ExternalAccountDataModified + } else { + InstructionError::ReadonlyDataModified + }, + )); } } } diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index 94046f5f741560..eac82d2cfeecdd 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -3,7 +3,9 @@ use { crate::serialization::account_data_region_memory_state, scopeguard::defer, solana_measure::measure::Measure, - solana_program_runtime::invoke_context::SerializedAccountMetadata, + solana_program_runtime::{ + invoke_context::SerializedAccountMetadata, loaded_programs::ProgramCacheEntryType, + }, solana_rbpf::{ ebpf, memory_region::{MemoryRegion, MemoryState}, @@ -866,6 +868,9 @@ where let direct_mapping = invoke_context .get_feature_set() .is_active(&feature_set::bpf_account_data_direct_mapping::id()); + let infer_account_is_executable_flag = invoke_context + .get_feature_set() + .is_active(&feature_set::infer_account_is_executable_flag::id()); for (instruction_account_index, instruction_account) in instruction_accounts.iter().enumerate() { @@ -881,7 +886,16 @@ where .transaction_context .get_key_of_account_at_index(instruction_account.index_in_transaction)?; - if callee_account.is_executable() { + let is_executable = if infer_account_is_executable_flag { + invoke_context + .program_cache_for_tx_batch + .find(callee_account.get_key()) + .map(|entry| !matches!(entry.program, ProgramCacheEntryType::Closed)) + .unwrap_or(false) + } else { + callee_account.is_executable() + }; + if is_executable { // Use the known account consume_compute_meter( invoke_context, diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index 0bb2f0ec983659..b105700a9fedbd 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -143,6 +143,8 @@ pub struct TransactionContext { return_data: TransactionReturnData, accounts_resize_delta: RefCell, #[cfg(not(target_os = "solana"))] + infer_account_is_executable_flag: bool, + #[cfg(not(target_os = "solana"))] rent: Rent, /// Useful for debugging to filter by or to look it up on the explorer #[cfg(all(not(target_os = "solana"), feature = "full", debug_assertions))] @@ -171,12 +173,18 @@ impl TransactionContext { instruction_trace: vec![InstructionContext::default()], return_data: TransactionReturnData::default(), accounts_resize_delta: RefCell::new(0), + infer_account_is_executable_flag: true, rent, #[cfg(all(not(target_os = "solana"), feature = "full", debug_assertions))] signature: Signature::default(), } } + #[cfg(not(target_os = "solana"))] + pub fn set_infer_account_is_executable_flag(&mut self, enabled: bool) { + self.infer_account_is_executable_flag = enabled; + } + /// Used in mock_process_instruction #[cfg(not(target_os = "solana"))] pub fn deconstruct_without_keys(self) -> Result, InstructionError> { @@ -996,7 +1004,14 @@ impl<'a> BorrowedAccount<'a> { /// Returns whether this account is executable (transaction wide) #[inline] pub fn is_executable(&self) -> bool { - self.account.executable() + #[cfg(target_os = "solana")] + { + self.account.executable() + } + #[cfg(not(target_os = "solana"))] + { + !self.transaction_context.infer_account_is_executable_flag && self.account.executable() + } } /// Configures whether this account is executable (transaction wide) diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index c62963c09e0934..65bbc8e3d41c01 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -314,7 +314,9 @@ fn load_transaction_accounts( return Err(TransactionError::ProgramAccountNotFound); } - if !program_account.executable() { + if !feature_set.is_active(&feature_set::infer_account_is_executable_flag::id()) + && !program_account.executable() + { error_metrics.invalid_program_for_execution += 1; return Err(TransactionError::InvalidProgramForExecution); } @@ -334,7 +336,9 @@ fn load_transaction_accounts( let owner_index = accounts.len(); if let Some(owner_account) = callbacks.get_account_shared_data(owner_id) { if !native_loader::check_id(owner_account.owner()) - || !owner_account.executable() + || (!feature_set + .is_active(&feature_set::infer_account_is_executable_flag::id()) + && !owner_account.executable()) { error_metrics.invalid_program_for_execution += 1; return Err(TransactionError::InvalidProgramForExecution); diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs index 73afc8cac772db..9937c23fd86596 100644 --- a/svm/src/transaction_processor.rs +++ b/svm/src/transaction_processor.rs @@ -37,7 +37,7 @@ use { account::{AccountSharedData, ReadableAccount, PROGRAM_OWNERS}, clock::{Epoch, Slot}, feature_set::{ - include_loaded_accounts_data_size_in_fee_calculation, + include_loaded_accounts_data_size_in_fee_calculation, infer_account_is_executable_flag, remove_rounding_in_fee_calculation, FeatureSet, }, fee::{FeeBudgetLimits, FeeStructure}, @@ -744,6 +744,11 @@ impl TransactionBatchProcessor { compute_budget.max_instruction_stack_depth, compute_budget.max_instruction_trace_length, ); + transaction_context.set_infer_account_is_executable_flag( + environment + .feature_set + .is_active(&infer_account_is_executable_flag::id()), + ); #[cfg(debug_assertions)] transaction_context.set_signature(tx.signature()); From 5cffa5c709eb387d7b444a22827c2ac202f74c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 10 Jul 2024 15:51:05 +0200 Subject: [PATCH 4/5] Adds deprecation attributes. --- program-runtime/src/invoke_context.rs | 1 + programs/bpf_loader/src/lib.rs | 3 +++ programs/bpf_loader/src/serialization.rs | 1 + programs/bpf_loader/src/syscalls/cpi.rs | 1 + sdk/src/transaction_context.rs | 6 ++++++ 5 files changed, 12 insertions(+) diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index b587dd4a9fc979..e7b36ac00d04c1 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -434,6 +434,7 @@ impl<'a> InvokeContext<'a> { })?; let borrowed_program_account = instruction_context .try_borrow_instruction_account(self.transaction_context, program_account_index)?; + #[allow(deprecated)] if !self .get_feature_set() .is_active(&infer_account_is_executable_flag::id()) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 246c6703365b90..6db74a50553794 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -431,6 +431,7 @@ pub fn process_instruction_inner( } // Program Invocation + #[allow(deprecated)] if !invoke_context .get_feature_set() .is_active(&infer_account_is_executable_flag::id()) @@ -723,6 +724,7 @@ fn process_loader_upgradeable_instruction( let program = instruction_context.try_borrow_instruction_account(transaction_context, 1)?; + #[allow(deprecated)] if !invoke_context .get_feature_set() .is_active(&infer_account_is_executable_flag::id()) @@ -1471,6 +1473,7 @@ fn execute<'a, 'b: 'a>( )?; error = EbpfError::SyscallError(Box::new( + #[allow(deprecated)] if !invoke_context .get_feature_set() .is_active(&infer_account_is_executable_flag::id()) diff --git a/programs/bpf_loader/src/serialization.rs b/programs/bpf_loader/src/serialization.rs index bdfd98fdc9d9d9..220845adbc64c6 100644 --- a/programs/bpf_loader/src/serialization.rs +++ b/programs/bpf_loader/src/serialization.rs @@ -193,6 +193,7 @@ impl<'a> Serializer<'a> { .map(|entry| !matches!(entry.program, ProgramCacheEntryType::Closed)) .unwrap_or(false) } else { + #[allow(deprecated)] account.is_executable() } } diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index eac82d2cfeecdd..6adef20c9ab69a 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -893,6 +893,7 @@ where .map(|entry| !matches!(entry.program, ProgramCacheEntryType::Closed)) .unwrap_or(false) } else { + #[allow(deprecated)] callee_account.is_executable() }; if is_executable { diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index b105700a9fedbd..bee9c9511bfb63 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -757,6 +757,7 @@ impl<'a> BorrowedAccount<'a> { return Err(InstructionError::ModifiedProgramId); } // and only if the account is not executable + #[allow(deprecated)] if self.is_executable() { return Err(InstructionError::ModifiedProgramId); } @@ -791,6 +792,7 @@ impl<'a> BorrowedAccount<'a> { return Err(InstructionError::ReadonlyLamportChange); } // The balance of executable accounts may not change + #[allow(deprecated)] if self.is_executable() { return Err(InstructionError::ExecutableLamportChange); } @@ -1003,6 +1005,7 @@ impl<'a> BorrowedAccount<'a> { /// Returns whether this account is executable (transaction wide) #[inline] + #[deprecated(since = "2.1.0", note = "Use `get_owner` instead")] pub fn is_executable(&self) -> bool { #[cfg(target_os = "solana")] { @@ -1034,10 +1037,12 @@ impl<'a> BorrowedAccount<'a> { return Err(InstructionError::ExecutableModified); } // one can not clear the executable flag + #[allow(deprecated)] if self.is_executable() && !is_executable { return Err(InstructionError::ExecutableModified); } // don't touch the account if the executable flag does not change + #[allow(deprecated)] if self.is_executable() == is_executable { return Ok(()); } @@ -1091,6 +1096,7 @@ impl<'a> BorrowedAccount<'a> { #[cfg(not(target_os = "solana"))] pub fn can_data_be_changed(&self) -> Result<(), InstructionError> { // Only non-executable accounts data can be changed + #[allow(deprecated)] if self.is_executable() { return Err(InstructionError::ExecutableDataModified); } From b158a1e2713e4dfa4c1808a98c16dddb257dfde6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 10 Jul 2024 10:48:01 +0200 Subject: [PATCH 5/5] Adjusts tests. --- programs/bpf_loader/src/lib.rs | 26 +++++++++++++++++++++++--- programs/sbf/tests/programs.rs | 10 +++++----- svm/src/account_loader.rs | 10 +++++++++- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index 6db74a50553794..496f524e169c78 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -1723,13 +1723,21 @@ mod tests { // Case: Account not a program program_account.set_executable(false); - process_instruction( + mock_process_instruction( &loader_id, - &[0], + vec![0], &[], vec![(program_id, program_account)], Vec::new(), Err(InstructionError::IncorrectProgramId), + Entrypoint::vm, + |invoke_context| { + let mut feature_set = invoke_context.get_feature_set().clone(); + feature_set.deactivate(&infer_account_is_executable_flag::id()); + invoke_context.mock_set_feature_set(Arc::new(feature_set)); + test_utils::load_all_invoked_programs(invoke_context); + }, + |_invoke_context| {}, ); } @@ -2404,10 +2412,22 @@ mod tests { .unwrap() .1 .set_executable(false); - process_instruction( + let instruction_data = bincode::serialize(&UpgradeableLoaderInstruction::Upgrade).unwrap(); + mock_process_instruction( + &bpf_loader_upgradeable::id(), + Vec::new(), + &instruction_data, transaction_accounts, instruction_accounts, Err(InstructionError::AccountNotExecutable), + Entrypoint::vm, + |invoke_context| { + let mut feature_set = invoke_context.get_feature_set().clone(); + feature_set.deactivate(&infer_account_is_executable_flag::id()); + invoke_context.mock_set_feature_set(Arc::new(feature_set)); + test_utils::load_all_invoked_programs(invoke_context); + }, + |_invoke_context| {}, ); // Case: Program account now owned by loader diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs index 729bcb26de0e73..e10a7cde0e0c8f 100644 --- a/programs/sbf/tests/programs.rs +++ b/programs/sbf/tests/programs.rs @@ -927,15 +927,15 @@ fn test_program_sbf_invoke_sanity() { do_invoke_failure_test_local( TEST_PPROGRAM_NOT_OWNED_BY_LOADER, - TransactionError::InstructionError(0, InstructionError::AccountNotExecutable), - &[], + TransactionError::InstructionError(0, InstructionError::UnsupportedProgramId), + &[argument_keypair.pubkey()], None, ); do_invoke_failure_test_local( TEST_PPROGRAM_NOT_EXECUTABLE, - TransactionError::InstructionError(0, InstructionError::AccountNotExecutable), - &[], + TransactionError::InstructionError(0, InstructionError::InvalidAccountData), + &[unexecutable_program_keypair.pubkey()], None, ); @@ -4653,7 +4653,7 @@ fn test_deny_executable_write() { let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction); assert_eq!( result.unwrap_err().unwrap(), - TransactionError::InstructionError(0, InstructionError::ExecutableDataModified) + TransactionError::InstructionError(0, InstructionError::ReadonlyDataModified) ); } } diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs index 65bbc8e3d41c01..71632e3d882299 100644 --- a/svm/src/account_loader.rs +++ b/svm/src/account_loader.rs @@ -701,7 +701,15 @@ mod tests { instructions, ); - let loaded_accounts = load_accounts_aux_test(tx, &accounts, &mut error_metrics); + let mut feature_set = FeatureSet::all_enabled(); + feature_set.deactivate(&feature_set::infer_account_is_executable_flag::id()); + let loaded_accounts = load_accounts_with_features_and_rent( + tx, + &accounts, + &RentCollector::default(), + &mut error_metrics, + &mut feature_set, + ); assert_eq!(error_metrics.invalid_program_for_execution, 1); assert_eq!(loaded_accounts.len(), 1);