From 1069ed739c9149e6106793be2bdb332c397d027f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Sat, 7 Oct 2023 09:53:16 +0200 Subject: [PATCH] Cleans up the feature gate of enable_early_verification_of_account_modifications: - Removes PreAccount - Removes InvokeContext::pre_accounts and InvokeContext::rent - Removes InvokeContext::verify() and InvokeContext::verify_and_update() - Removes TransactionContext::is_early_verification_of_account_modifications_enabled() - Removes TransactionAccounts::is_early_verification_of_account_modifications_enabled - No longer optional: TransactionContext::rent --- accounts-db/src/transaction_results.rs | 7 +- program-runtime/benches/pre_account.rs | 101 --- program-runtime/src/invoke_context.rs | 291 +-------- program-runtime/src/lib.rs | 1 - program-runtime/src/message_processor.rs | 20 +- program-runtime/src/pre_account.rs | 610 ------------------- programs/bpf_loader/benches/serialization.rs | 2 +- programs/bpf_loader/src/syscalls/cpi.rs | 11 +- programs/bpf_loader/src/syscalls/mod.rs | 3 +- programs/vote/src/vote_state/mod.rs | 2 +- runtime/src/bank.rs | 31 +- runtime/src/bank/tests.rs | 3 +- sdk/src/transaction_context.rs | 211 +++---- 13 files changed, 111 insertions(+), 1182 deletions(-) delete mode 100644 program-runtime/benches/pre_account.rs delete mode 100644 program-runtime/src/pre_account.rs diff --git a/accounts-db/src/transaction_results.rs b/accounts-db/src/transaction_results.rs index f7228dfff5db88..7a6401d62d7a04 100644 --- a/accounts-db/src/transaction_results.rs +++ b/accounts-db/src/transaction_results.rs @@ -176,13 +176,16 @@ pub fn inner_instructions_list_from_instruction_trace( #[cfg(test)] mod tests { - use {super::*, solana_sdk::transaction_context::TransactionContext}; + use { + super::*, + solana_sdk::{sysvar::rent::Rent, transaction_context::TransactionContext}, + }; #[test] fn test_inner_instructions_list_from_instruction_trace() { let instruction_trace = [1, 2, 1, 1, 2, 3, 2]; let mut transaction_context = - TransactionContext::new(vec![], None, 3, instruction_trace.len()); + TransactionContext::new(vec![], Rent::default(), 3, instruction_trace.len()); for (index_in_trace, stack_height) in instruction_trace.into_iter().enumerate() { while stack_height <= transaction_context.get_instruction_context_stack_height() { transaction_context.pop().unwrap(); diff --git a/program-runtime/benches/pre_account.rs b/program-runtime/benches/pre_account.rs deleted file mode 100644 index b5fdc5cb945e3e..00000000000000 --- a/program-runtime/benches/pre_account.rs +++ /dev/null @@ -1,101 +0,0 @@ -#![feature(test)] - -extern crate test; - -use { - log::*, - solana_program_runtime::{pre_account::PreAccount, timings::ExecuteDetailsTimings}, - solana_sdk::{account::AccountSharedData, pubkey, rent::Rent}, - test::Bencher, -}; - -#[bench] -fn bench_verify_account_changes_data(bencher: &mut Bencher) { - solana_logger::setup(); - - let owner = pubkey::new_rand(); - let non_owner = pubkey::new_rand(); - let pre = PreAccount::new( - &pubkey::new_rand(), - AccountSharedData::new(0, BUFSIZE, &owner), - ); - let post = AccountSharedData::new(0, BUFSIZE, &owner); - assert_eq!( - pre.verify( - &owner, - false, - &Rent::default(), - &post, - &mut ExecuteDetailsTimings::default(), - false, - ), - Ok(()) - ); - - // this one should be faster - bencher.iter(|| { - pre.verify( - &owner, - false, - &Rent::default(), - &post, - &mut ExecuteDetailsTimings::default(), - false, - ) - .unwrap(); - }); - let summary = bencher.bench(|_bencher| Ok(())).unwrap().unwrap(); - info!("data no change by owner: {} ns/iter", summary.median); - - let pre_data = vec![BUFSIZE]; - let post_data = vec![BUFSIZE]; - bencher.iter(|| pre_data == post_data); - let summary = bencher.bench(|_bencher| Ok(())).unwrap().unwrap(); - info!("data compare {} ns/iter", summary.median); - - let pre = PreAccount::new( - &pubkey::new_rand(), - AccountSharedData::new(0, BUFSIZE, &owner), - ); - bencher.iter(|| { - pre.verify( - &non_owner, - false, - &Rent::default(), - &post, - &mut ExecuteDetailsTimings::default(), - false, - ) - .unwrap(); - }); - let summary = bencher.bench(|_bencher| Ok(())).unwrap().unwrap(); - info!("data no change by non owner: {} ns/iter", summary.median); -} - -const BUFSIZE: usize = 1024 * 1024 + 127; -static BUF0: [u8; BUFSIZE] = [0; BUFSIZE]; -static BUF1: [u8; BUFSIZE] = [1; BUFSIZE]; - -#[bench] -fn bench_is_zeroed(bencher: &mut Bencher) { - bencher.iter(|| { - PreAccount::is_zeroed(&BUF0); - }); -} - -#[bench] -fn bench_is_zeroed_not(bencher: &mut Bencher) { - bencher.iter(|| { - PreAccount::is_zeroed(&BUF1); - }); -} - -#[bench] -fn bench_is_zeroed_by_iter(bencher: &mut Bencher) { - bencher.iter(|| BUF0.iter().all(|item| *item == 0)); -} - -#[bench] -fn bench_is_zeroed_not_by_iter(bencher: &mut Bencher) { - bencher.iter(|| BUF1.iter().all(|item| *item == 0)); -} diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 12f82300d78521..abe49ccd84b270 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -2,10 +2,9 @@ use { crate::{ accounts_data_meter::AccountsDataMeter, compute_budget::ComputeBudget, - ic_logger_msg, ic_msg, + ic_msg, loaded_programs::{LoadedProgram, LoadedProgramType, LoadedProgramsForTxBatch}, log_collector::LogCollector, - pre_account::PreAccount, stable_log, sysvar_cache::SysvarCache, timings::{ExecuteDetailsTimings, ExecuteTimings}, @@ -18,17 +17,13 @@ use { vm::{BuiltinFunction, Config, ContextObject, ProgramResult}, }, solana_sdk::{ - account::{AccountSharedData, ReadableAccount}, + account::AccountSharedData, bpf_loader_deprecated, - feature_set::{ - check_slice_translation_size, enable_early_verification_of_account_modifications, - native_programs_consume_cu, FeatureSet, - }, + feature_set::{check_slice_translation_size, native_programs_consume_cu, FeatureSet}, hash::Hash, instruction::{AccountMeta, InstructionError}, native_loader, pubkey::Pubkey, - rent::Rent, saturating_add_assign, stable_layout::stable_instruction::StableInstruction, transaction_context::{ @@ -161,8 +156,6 @@ pub struct SerializedAccountMetadata { pub struct InvokeContext<'a> { pub transaction_context: &'a mut TransactionContext, - rent: Rent, - pre_accounts: Vec, sysvar_cache: &'a SysvarCache, log_collector: Option>>, compute_budget: ComputeBudget, @@ -184,7 +177,6 @@ impl<'a> InvokeContext<'a> { #[allow(clippy::too_many_arguments)] pub fn new( transaction_context: &'a mut TransactionContext, - rent: Rent, sysvar_cache: &'a SysvarCache, log_collector: Option>>, compute_budget: ComputeBudget, @@ -198,8 +190,6 @@ impl<'a> InvokeContext<'a> { ) -> Self { Self { transaction_context, - rent, - pre_accounts: Vec::new(), sysvar_cache, log_collector, current_compute_budget: compute_budget, @@ -242,42 +232,6 @@ impl<'a> InvokeContext<'a> { == 0 { self.current_compute_budget = self.compute_budget; - - if !self - .feature_set - .is_active(&enable_early_verification_of_account_modifications::id()) - { - self.pre_accounts = Vec::with_capacity( - instruction_context.get_number_of_instruction_accounts() as usize, - ); - for instruction_account_index in - 0..instruction_context.get_number_of_instruction_accounts() - { - if instruction_context - .is_instruction_account_duplicate(instruction_account_index)? - .is_some() - { - continue; // Skip duplicate account - } - let index_in_transaction = instruction_context - .get_index_of_instruction_account_in_transaction( - instruction_account_index, - )?; - if index_in_transaction >= self.transaction_context.get_number_of_accounts() { - return Err(InstructionError::MissingAccount); - } - let account = self - .transaction_context - .get_account_at_index(index_in_transaction)? - .borrow() - .clone(); - self.pre_accounts.push(PreAccount::new( - self.transaction_context - .get_key_of_account_at_index(index_in_transaction)?, - account, - )); - } - } } else { let contains = (0..self .transaction_context @@ -325,189 +279,6 @@ impl<'a> InvokeContext<'a> { .get_instruction_context_stack_height() } - /// Verify the results of an instruction - /// - /// Note: `instruction_accounts` must be the same as passed to `InvokeContext::push()`, - /// so that they match the order of `pre_accounts`. - fn verify( - &mut self, - instruction_accounts: &[InstructionAccount], - program_indices: &[IndexOfAccount], - ) -> Result<(), InstructionError> { - let instruction_context = self - .transaction_context - .get_current_instruction_context() - .map_err(|_| InstructionError::CallDepth)?; - let program_id = instruction_context - .get_last_program_key(self.transaction_context) - .map_err(|_| InstructionError::CallDepth)?; - - // Verify all executable accounts have zero outstanding refs - for account_index in program_indices.iter() { - self.transaction_context - .get_account_at_index(*account_index)? - .try_borrow_mut() - .map_err(|_| InstructionError::AccountBorrowOutstanding)?; - } - - // Verify the per-account instruction results - let (mut pre_sum, mut post_sum) = (0_u128, 0_u128); - let mut pre_account_index = 0; - for (instruction_account_index, instruction_account) in - instruction_accounts.iter().enumerate() - { - if instruction_account_index as IndexOfAccount != instruction_account.index_in_callee { - continue; // Skip duplicate account - } - { - // Verify account has no outstanding references - let _ = self - .transaction_context - .get_account_at_index(instruction_account.index_in_transaction)? - .try_borrow_mut() - .map_err(|_| InstructionError::AccountBorrowOutstanding)?; - } - let pre_account = &self - .pre_accounts - .get(pre_account_index) - .ok_or(InstructionError::NotEnoughAccountKeys)?; - pre_account_index = pre_account_index.saturating_add(1); - let account = self - .transaction_context - .get_account_at_index(instruction_account.index_in_transaction)? - .borrow(); - pre_account - .verify( - program_id, - instruction_account.is_writable, - &self.rent, - &account, - &mut self.timings, - true, - ) - .map_err(|err| { - ic_logger_msg!( - self.log_collector, - "failed to verify account {}: {}", - pre_account.key(), - err - ); - err - })?; - pre_sum = pre_sum - .checked_add(u128::from(pre_account.lamports())) - .ok_or(InstructionError::UnbalancedInstruction)?; - post_sum = post_sum - .checked_add(u128::from(account.lamports())) - .ok_or(InstructionError::UnbalancedInstruction)?; - - let pre_data_len = pre_account.data().len() as i64; - let post_data_len = account.data().len() as i64; - let data_len_delta = post_data_len.saturating_sub(pre_data_len); - self.accounts_data_meter - .adjust_delta_unchecked(data_len_delta); - } - - // Verify that the total sum of all the lamports did not change - if pre_sum != post_sum { - return Err(InstructionError::UnbalancedInstruction); - } - Ok(()) - } - - /// Verify and update PreAccount state based on program execution - /// - /// Note: `instruction_accounts` must be the same as passed to `InvokeContext::push()`, - /// so that they match the order of `pre_accounts`. - fn verify_and_update( - &mut self, - instruction_accounts: &[InstructionAccount], - before_instruction_context_push: bool, - ) -> Result<(), InstructionError> { - let transaction_context = &self.transaction_context; - let instruction_context = transaction_context.get_current_instruction_context()?; - let program_id = instruction_context - .get_last_program_key(transaction_context) - .map_err(|_| InstructionError::CallDepth)?; - - // Verify the per-account instruction results - let (mut pre_sum, mut post_sum) = (0_u128, 0_u128); - for (instruction_account_index, instruction_account) in - instruction_accounts.iter().enumerate() - { - if instruction_account_index as IndexOfAccount != instruction_account.index_in_callee { - continue; // Skip duplicate account - } - if instruction_account.index_in_transaction - < transaction_context.get_number_of_accounts() - { - let key = transaction_context - .get_key_of_account_at_index(instruction_account.index_in_transaction)?; - let account = transaction_context - .get_account_at_index(instruction_account.index_in_transaction)?; - let is_writable = if before_instruction_context_push { - instruction_context - .is_instruction_account_writable(instruction_account.index_in_caller)? - } else { - instruction_account.is_writable - }; - // Find the matching PreAccount - for pre_account in self.pre_accounts.iter_mut() { - if key == pre_account.key() { - { - // Verify account has no outstanding references - let _ = account - .try_borrow_mut() - .map_err(|_| InstructionError::AccountBorrowOutstanding)?; - } - let account = account.borrow(); - pre_account - .verify( - program_id, - is_writable, - &self.rent, - &account, - &mut self.timings, - false, - ) - .map_err(|err| { - ic_logger_msg!( - self.log_collector, - "failed to verify account {}: {}", - key, - err - ); - err - })?; - pre_sum = pre_sum - .checked_add(u128::from(pre_account.lamports())) - .ok_or(InstructionError::UnbalancedInstruction)?; - post_sum = post_sum - .checked_add(u128::from(account.lamports())) - .ok_or(InstructionError::UnbalancedInstruction)?; - if is_writable && !pre_account.executable() { - pre_account.update(account.clone()); - } - - let pre_data_len = pre_account.data().len() as i64; - let post_data_len = account.data().len() as i64; - let data_len_delta = post_data_len.saturating_sub(pre_data_len); - self.accounts_data_meter - .adjust_delta_unchecked(data_len_delta); - - break; - } - } - } - } - - // Verify that the total sum of all the lamports did not change - if pre_sum != post_sum { - return Err(InstructionError::UnbalancedInstruction); - } - Ok(()) - } - /// Entrypoint for a cross-program invocation from a builtin program pub fn native_invoke( &mut self, @@ -660,60 +431,11 @@ impl<'a> InvokeContext<'a> { timings: &mut ExecuteTimings, ) -> Result<(), InstructionError> { *compute_units_consumed = 0; - - let nesting_level = self - .transaction_context - .get_instruction_context_stack_height(); - let is_top_level_instruction = nesting_level == 0; - if !is_top_level_instruction - && !self - .feature_set - .is_active(&enable_early_verification_of_account_modifications::id()) - { - // Verify the calling program hasn't misbehaved - let mut verify_caller_time = Measure::start("verify_caller_time"); - let verify_caller_result = self.verify_and_update(instruction_accounts, true); - verify_caller_time.stop(); - saturating_add_assign!( - timings - .execute_accessories - .process_instructions - .verify_caller_us, - verify_caller_time.as_us() - ); - verify_caller_result?; - } - self.transaction_context .get_next_instruction_context()? .configure(program_indices, instruction_accounts, instruction_data); self.push()?; self.process_executable_chain(compute_units_consumed, timings) - .and_then(|_| { - if self - .feature_set - .is_active(&enable_early_verification_of_account_modifications::id()) - { - Ok(()) - } else { - // Verify the called program has not misbehaved - let mut verify_callee_time = Measure::start("verify_callee_time"); - let result = if is_top_level_instruction { - self.verify(instruction_accounts, program_indices) - } else { - self.verify_and_update(instruction_accounts, false) - }; - verify_callee_time.stop(); - saturating_add_assign!( - timings - .execute_accessories - .process_instructions - .verify_callee_us, - verify_callee_time.as_us() - ); - result - } - }) // MUST pop if and only if `push` succeeded, independent of `result`. // Thus, the `.and()` instead of an `.and_then()`. .and(self.pop()) @@ -929,7 +651,7 @@ macro_rules! with_mock_invoke_context { let compute_budget = ComputeBudget::default(); let mut $transaction_context = TransactionContext::new( $transaction_accounts, - Some(Rent::default()), + Rent::default(), compute_budget.max_invoke_stack_height, compute_budget.max_instruction_trace_length, ); @@ -957,7 +679,6 @@ macro_rules! with_mock_invoke_context { let mut programs_updated_only_for_global_cache = LoadedProgramsForTxBatch::default(); let mut $invoke_context = InvokeContext::new( &mut $transaction_context, - Rent::default(), &sysvar_cache, Some(LogCollector::new_ref()), compute_budget, @@ -1038,7 +759,7 @@ mod tests { super::*, crate::compute_budget, serde::{Deserialize, Serialize}, - solana_sdk::{account::WritableAccount, instruction::Instruction}, + solana_sdk::{account::WritableAccount, instruction::Instruction, rent::Rent}, }; #[derive(Debug, Serialize, Deserialize)] @@ -1223,7 +944,7 @@ mod tests { fn test_max_instruction_trace_length() { const MAX_INSTRUCTIONS: usize = 8; let mut transaction_context = - TransactionContext::new(Vec::new(), Some(Rent::default()), 1, MAX_INSTRUCTIONS); + TransactionContext::new(Vec::new(), Rent::default(), 1, MAX_INSTRUCTIONS); for _ in 0..MAX_INSTRUCTIONS { transaction_context.push().unwrap(); transaction_context.pop().unwrap(); diff --git a/program-runtime/src/lib.rs b/program-runtime/src/lib.rs index d43d9a5b35fafc..c79505495f3e73 100644 --- a/program-runtime/src/lib.rs +++ b/program-runtime/src/lib.rs @@ -16,7 +16,6 @@ pub mod invoke_context; pub mod loaded_programs; pub mod log_collector; pub mod message_processor; -pub mod pre_account; pub mod prioritization_fee; pub mod stable_log; pub mod sysvar_cache; diff --git a/program-runtime/src/message_processor.rs b/program-runtime/src/message_processor.rs index 80bfaf16e974bc..6b3727e6a8ce74 100644 --- a/program-runtime/src/message_processor.rs +++ b/program-runtime/src/message_processor.rs @@ -15,7 +15,6 @@ use { hash::Hash, message::SanitizedMessage, precompiles::is_precompile, - rent::Rent, saturating_add_assign, sysvar::instructions, transaction::TransactionError, @@ -54,7 +53,6 @@ impl MessageProcessor { message: &SanitizedMessage, program_indices: &[Vec], transaction_context: &mut TransactionContext, - rent: Rent, log_collector: Option>>, programs_loaded_for_tx_batch: &LoadedProgramsForTxBatch, programs_modified_by_tx: &mut LoadedProgramsForTxBatch, @@ -70,7 +68,6 @@ impl MessageProcessor { ) -> Result { let mut invoke_context = InvokeContext::new( transaction_context, - rent, sysvar_cache, log_collector, compute_budget, @@ -199,6 +196,7 @@ mod tests { message::{AccountKeys, LegacyMessage, Message}, native_loader::{self, create_loadable_account_for_test}, pubkey::Pubkey, + rent::Rent, secp256k1_instruction::new_secp256k1_instruction, secp256k1_program, }, @@ -268,8 +266,7 @@ mod tests { create_loadable_account_for_test("mock_system_program"), ), ]; - let mut transaction_context = - TransactionContext::new(accounts, Some(Rent::default()), 1, 3); + let mut transaction_context = TransactionContext::new(accounts, Rent::default(), 1, 3); let program_indices = vec![vec![2]]; let mut programs_loaded_for_tx_batch = LoadedProgramsForTxBatch::default(); programs_loaded_for_tx_batch.replenish( @@ -310,7 +307,6 @@ mod tests { &message, &program_indices, &mut transaction_context, - Rent::default(), None, &programs_loaded_for_tx_batch, &mut programs_modified_by_tx, @@ -363,7 +359,6 @@ mod tests { &message, &program_indices, &mut transaction_context, - Rent::default(), None, &programs_loaded_for_tx_batch, &mut programs_modified_by_tx, @@ -406,7 +401,6 @@ mod tests { &message, &program_indices, &mut transaction_context, - Rent::default(), None, &programs_loaded_for_tx_batch, &mut programs_modified_by_tx, @@ -501,8 +495,7 @@ mod tests { create_loadable_account_for_test("mock_system_program"), ), ]; - let mut transaction_context = - TransactionContext::new(accounts, Some(Rent::default()), 1, 3); + let mut transaction_context = TransactionContext::new(accounts, Rent::default(), 1, 3); let program_indices = vec![vec![2]]; let mut programs_loaded_for_tx_batch = LoadedProgramsForTxBatch::default(); programs_loaded_for_tx_batch.replenish( @@ -540,7 +533,6 @@ mod tests { &message, &program_indices, &mut transaction_context, - Rent::default(), None, &programs_loaded_for_tx_batch, &mut programs_modified_by_tx, @@ -577,7 +569,6 @@ mod tests { &message, &program_indices, &mut transaction_context, - Rent::default(), None, &programs_loaded_for_tx_batch, &mut programs_modified_by_tx, @@ -611,7 +602,6 @@ mod tests { &message, &program_indices, &mut transaction_context, - Rent::default(), None, &programs_loaded_for_tx_batch, &mut programs_modified_by_tx, @@ -667,8 +657,7 @@ mod tests { (secp256k1_program::id(), secp256k1_account), (mock_program_id, mock_program_account), ]; - let mut transaction_context = - TransactionContext::new(accounts, Some(Rent::default()), 1, 2); + let mut transaction_context = TransactionContext::new(accounts, Rent::default(), 1, 2); // Since libsecp256k1 is still using the old version of rand, this test // copies the `random` implementation at: @@ -703,7 +692,6 @@ mod tests { &message, &[vec![0], vec![1]], &mut transaction_context, - Rent::default(), None, &programs_loaded_for_tx_batch, &mut programs_modified_by_tx, diff --git a/program-runtime/src/pre_account.rs b/program-runtime/src/pre_account.rs deleted file mode 100644 index 2ca91ba0904b7d..00000000000000 --- a/program-runtime/src/pre_account.rs +++ /dev/null @@ -1,610 +0,0 @@ -use { - crate::timings::ExecuteDetailsTimings, - solana_sdk::{ - account::{AccountSharedData, ReadableAccount, WritableAccount}, - instruction::InstructionError, - pubkey::Pubkey, - rent::Rent, - system_instruction::MAX_PERMITTED_DATA_LENGTH, - }, - std::fmt::Debug, -}; - -// The relevant state of an account before an Instruction executes, used -// to verify account integrity after the Instruction completes -#[derive(Clone, Debug, Default)] -pub struct PreAccount { - key: Pubkey, - account: AccountSharedData, - changed: bool, -} -impl PreAccount { - pub fn new(key: &Pubkey, account: AccountSharedData) -> Self { - Self { - key: *key, - account, - changed: false, - } - } - - pub fn verify( - &self, - program_id: &Pubkey, - is_writable: bool, - rent: &Rent, - post: &AccountSharedData, - timings: &mut ExecuteDetailsTimings, - outermost_call: bool, - ) -> Result<(), InstructionError> { - let pre = &self.account; - - // Only the owner of the account may change owner and - // only if the account is writable and - // only if the account is not executable and - // only if the data is zero-initialized or empty - let owner_changed = pre.owner() != post.owner(); - if owner_changed - && (!is_writable // line coverage used to get branch coverage - || pre.executable() - || program_id != pre.owner() - || !Self::is_zeroed(post.data())) - { - return Err(InstructionError::ModifiedProgramId); - } - - // An account not assigned to the program cannot have its balance decrease. - if program_id != pre.owner() // line coverage used to get branch coverage - && pre.lamports() > post.lamports() - { - return Err(InstructionError::ExternalAccountLamportSpend); - } - - // The balance of read-only and executable accounts may not change - let lamports_changed = pre.lamports() != post.lamports(); - if lamports_changed { - if !is_writable { - return Err(InstructionError::ReadonlyLamportChange); - } - if pre.executable() { - return Err(InstructionError::ExecutableLamportChange); - } - } - - // Account data size cannot exceed a maxumum length - if post.data().len() > MAX_PERMITTED_DATA_LENGTH as usize { - return Err(InstructionError::InvalidRealloc); - } - - // The owner of the account can change the size of the data - let data_len_changed = pre.data().len() != post.data().len(); - if data_len_changed && program_id != pre.owner() { - return Err(InstructionError::AccountDataSizeChanged); - } - - // Only the owner may change account data - // and if the account is writable - // and if the account is not executable - if !(program_id == pre.owner() - && is_writable // line coverage used to get branch coverage - && !pre.executable()) - && pre.data() != post.data() - { - if pre.executable() { - return Err(InstructionError::ExecutableDataModified); - } else if is_writable { - return Err(InstructionError::ExternalAccountDataModified); - } else { - return Err(InstructionError::ReadonlyDataModified); - } - } - - // executable is one-way (false->true) and only the account owner may set it. - let executable_changed = pre.executable() != post.executable(); - if executable_changed { - if !rent.is_exempt(post.lamports(), post.data().len()) { - return Err(InstructionError::ExecutableAccountNotRentExempt); - } - if !is_writable // line coverage used to get branch coverage - || pre.executable() - || program_id != post.owner() - { - return Err(InstructionError::ExecutableModified); - } - } - - // No one modifies rent_epoch (yet). - let rent_epoch_changed = pre.rent_epoch() != post.rent_epoch(); - if rent_epoch_changed { - return Err(InstructionError::RentEpochModified); - } - - if outermost_call { - timings.total_account_count = timings.total_account_count.saturating_add(1); - if owner_changed - || lamports_changed - || data_len_changed - || executable_changed - || rent_epoch_changed - || self.changed - { - timings.changed_account_count = timings.changed_account_count.saturating_add(1); - } - } - - Ok(()) - } - - pub fn update(&mut self, account: AccountSharedData) { - let rent_epoch = self.account.rent_epoch(); - self.account = account; - self.account.set_rent_epoch(rent_epoch); - - self.changed = true; - } - - pub fn key(&self) -> &Pubkey { - &self.key - } - - pub fn data(&self) -> &[u8] { - self.account.data() - } - - pub fn lamports(&self) -> u64 { - self.account.lamports() - } - - pub fn executable(&self) -> bool { - self.account.executable() - } - - pub fn is_zeroed(buf: &[u8]) -> bool { - const ZEROS_LEN: usize = 1024; - static ZEROS: [u8; ZEROS_LEN] = [0; ZEROS_LEN]; - let mut chunks = buf.chunks_exact(ZEROS_LEN); - - #[allow(clippy::indexing_slicing)] - { - chunks.all(|chunk| chunk == &ZEROS[..]) - && chunks.remainder() == &ZEROS[..chunks.remainder().len()] - } - } -} - -#[cfg(test)] -mod tests { - use { - super::*, - solana_sdk::{account::Account, instruction::InstructionError, system_program}, - }; - - #[test] - fn test_is_zeroed() { - const ZEROS_LEN: usize = 1024; - let mut buf = [0; ZEROS_LEN]; - assert!(PreAccount::is_zeroed(&buf)); - buf[0] = 1; - assert!(!PreAccount::is_zeroed(&buf)); - - let mut buf = [0; ZEROS_LEN - 1]; - assert!(PreAccount::is_zeroed(&buf)); - buf[0] = 1; - assert!(!PreAccount::is_zeroed(&buf)); - - let mut buf = [0; ZEROS_LEN + 1]; - assert!(PreAccount::is_zeroed(&buf)); - buf[0] = 1; - assert!(!PreAccount::is_zeroed(&buf)); - - let buf = vec![]; - assert!(PreAccount::is_zeroed(&buf)); - } - - struct Change { - program_id: Pubkey, - is_writable: bool, - rent: Rent, - pre: PreAccount, - post: AccountSharedData, - } - impl Change { - pub fn new(owner: &Pubkey, program_id: &Pubkey) -> Self { - Self { - program_id: *program_id, - rent: Rent::default(), - is_writable: true, - pre: PreAccount::new( - &solana_sdk::pubkey::new_rand(), - AccountSharedData::from(Account { - owner: *owner, - lamports: std::u64::MAX, - ..Account::default() - }), - ), - post: AccountSharedData::from(Account { - owner: *owner, - lamports: std::u64::MAX, - ..Account::default() - }), - } - } - pub fn read_only(mut self) -> Self { - self.is_writable = false; - self - } - pub fn executable(mut self, pre: bool, post: bool) -> Self { - self.pre.account.set_executable(pre); - self.post.set_executable(post); - self - } - pub fn lamports(mut self, pre: u64, post: u64) -> Self { - self.pre.account.set_lamports(pre); - self.post.set_lamports(post); - self - } - pub fn owner(mut self, post: &Pubkey) -> Self { - self.post.set_owner(*post); - self - } - pub fn data(mut self, pre: Vec, post: Vec) -> Self { - self.pre.account.set_data(pre); - self.post.set_data(post); - self - } - pub fn rent_epoch(mut self, pre: u64, post: u64) -> Self { - self.pre.account.set_rent_epoch(pre); - self.post.set_rent_epoch(post); - self - } - pub fn verify(&self) -> Result<(), InstructionError> { - self.pre.verify( - &self.program_id, - self.is_writable, - &self.rent, - &self.post, - &mut ExecuteDetailsTimings::default(), - false, - ) - } - } - - #[test] - fn test_verify_account_changes_owner() { - let system_program_id = system_program::id(); - let alice_program_id = solana_sdk::pubkey::new_rand(); - let mallory_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&system_program_id, &system_program_id) - .owner(&alice_program_id) - .verify(), - Ok(()), - "system program should be able to change the account owner" - ); - assert_eq!( - Change::new(&system_program_id, &system_program_id) - .owner(&alice_program_id) - .read_only() - .verify(), - Err(InstructionError::ModifiedProgramId), - "system program should not be able to change the account owner of a read-only account" - ); - assert_eq!( - Change::new(&mallory_program_id, &system_program_id) - .owner(&alice_program_id) - .verify(), - Err(InstructionError::ModifiedProgramId), - "system program should not be able to change the account owner of a non-system account" - ); - assert_eq!( - Change::new(&mallory_program_id, &mallory_program_id) - .owner(&alice_program_id) - .verify(), - Ok(()), - "mallory should be able to change the account owner, if she leaves clear data" - ); - assert_eq!( - Change::new(&mallory_program_id, &mallory_program_id) - .owner(&alice_program_id) - .data(vec![42], vec![0]) - .verify(), - Ok(()), - "mallory should be able to change the account owner, if she leaves clear data" - ); - assert_eq!( - Change::new(&mallory_program_id, &mallory_program_id) - .owner(&alice_program_id) - .executable(true, true) - .data(vec![42], vec![0]) - .verify(), - Err(InstructionError::ModifiedProgramId), - "mallory should not be able to change the account owner, if the account executable" - ); - assert_eq!( - Change::new(&mallory_program_id, &mallory_program_id) - .owner(&alice_program_id) - .data(vec![42], vec![42]) - .verify(), - Err(InstructionError::ModifiedProgramId), - "mallory should not be able to inject data into the alice program" - ); - } - - #[test] - fn test_verify_account_changes_executable() { - let owner = solana_sdk::pubkey::new_rand(); - let mallory_program_id = solana_sdk::pubkey::new_rand(); - let system_program_id = system_program::id(); - - assert_eq!( - Change::new(&owner, &system_program_id) - .executable(false, true) - .verify(), - Err(InstructionError::ExecutableModified), - "system program can't change executable if system doesn't own the account" - ); - assert_eq!( - Change::new(&owner, &system_program_id) - .executable(true, true) - .data(vec![1], vec![2]) - .verify(), - Err(InstructionError::ExecutableDataModified), - "system program can't change executable data if system doesn't own the account" - ); - assert_eq!( - Change::new(&owner, &owner).executable(false, true).verify(), - Ok(()), - "owner should be able to change executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(false, true) - .read_only() - .verify(), - Err(InstructionError::ExecutableModified), - "owner can't modify executable of read-only accounts" - ); - assert_eq!( - Change::new(&owner, &owner).executable(true, false).verify(), - Err(InstructionError::ExecutableModified), - "owner program can't reverse executable" - ); - assert_eq!( - Change::new(&owner, &mallory_program_id) - .executable(false, true) - .verify(), - Err(InstructionError::ExecutableModified), - "malicious Mallory should not be able to change the account executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(false, true) - .data(vec![1], vec![2]) - .verify(), - Ok(()), - "account data can change in the same instruction that sets the bit" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(true, true) - .data(vec![1], vec![2]) - .verify(), - Err(InstructionError::ExecutableDataModified), - "owner should not be able to change an account's data once its marked executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(true, true) - .lamports(1, 2) - .verify(), - Err(InstructionError::ExecutableLamportChange), - "owner should not be able to add lamports once marked executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(true, true) - .lamports(1, 2) - .verify(), - Err(InstructionError::ExecutableLamportChange), - "owner should not be able to add lamports once marked executable" - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(true, true) - .lamports(2, 1) - .verify(), - Err(InstructionError::ExecutableLamportChange), - "owner should not be able to subtract lamports once marked executable" - ); - let data = vec![1; 100]; - let min_lamports = Rent::default().minimum_balance(data.len()); - assert_eq!( - Change::new(&owner, &owner) - .executable(false, true) - .lamports(0, min_lamports) - .data(data.clone(), data.clone()) - .verify(), - Ok(()), - ); - assert_eq!( - Change::new(&owner, &owner) - .executable(false, true) - .lamports(0, min_lamports - 1) - .data(data.clone(), data) - .verify(), - Err(InstructionError::ExecutableAccountNotRentExempt), - "owner should not be able to change an account's data once its marked executable" - ); - } - - #[test] - fn test_verify_account_changes_data_len() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&system_program::id(), &system_program::id()) - .data(vec![0], vec![0, 0]) - .verify(), - Ok(()), - "system program should be able to change the data len" - ); - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .data(vec![0], vec![0,0]) - .verify(), - Err(InstructionError::AccountDataSizeChanged), - "system program should not be able to change the data length of accounts it does not own" - ); - } - - #[test] - fn test_verify_account_changes_data() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - let mallory_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .data(vec![0], vec![42]) - .verify(), - Ok(()), - "alice program should be able to change the data" - ); - assert_eq!( - Change::new(&mallory_program_id, &alice_program_id) - .data(vec![0], vec![42]) - .verify(), - Err(InstructionError::ExternalAccountDataModified), - "non-owner mallory should not be able to change the account data" - ); - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .data(vec![0], vec![42]) - .read_only() - .verify(), - Err(InstructionError::ReadonlyDataModified), - "alice isn't allowed to touch a CO account" - ); - } - - #[test] - fn test_verify_account_changes_rent_epoch() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &system_program::id()).verify(), - Ok(()), - "nothing changed!" - ); - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .rent_epoch(0, 1) - .verify(), - Err(InstructionError::RentEpochModified), - "no one touches rent_epoch" - ); - } - - #[test] - fn test_verify_account_changes_deduct_lamports_and_reassign_account() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - let bob_program_id = solana_sdk::pubkey::new_rand(); - - // positive test of this capability - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .owner(&bob_program_id) - .lamports(42, 1) - .data(vec![42], vec![0]) - .verify(), - Ok(()), - "alice should be able to deduct lamports and give the account to bob if the data is zeroed", - ); - } - - #[test] - fn test_verify_account_changes_lamports() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .lamports(42, 0) - .read_only() - .verify(), - Err(InstructionError::ExternalAccountLamportSpend), - "debit should fail, even if system program" - ); - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .lamports(42, 0) - .read_only() - .verify(), - Err(InstructionError::ReadonlyLamportChange), - "debit should fail, even if owning program" - ); - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .lamports(42, 0) - .owner(&system_program::id()) - .verify(), - Err(InstructionError::ModifiedProgramId), - "system program can't debit the account unless it was the pre.owner" - ); - assert_eq!( - Change::new(&system_program::id(), &system_program::id()) - .lamports(42, 0) - .owner(&alice_program_id) - .verify(), - Ok(()), - "system can spend (and change owner)" - ); - } - - #[test] - fn test_verify_account_changes_data_size_changed() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &system_program::id()) - .data(vec![0], vec![0, 0]) - .verify(), - Err(InstructionError::AccountDataSizeChanged), - "system program should not be able to change another program's account data size" - ); - assert_eq!( - Change::new(&alice_program_id, &solana_sdk::pubkey::new_rand()) - .data(vec![0], vec![0, 0]) - .verify(), - Err(InstructionError::AccountDataSizeChanged), - "one program should not be able to change another program's account data size" - ); - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .data(vec![0], vec![0, 0]) - .verify(), - Ok(()), - "programs can change their own data size" - ); - assert_eq!( - Change::new(&system_program::id(), &system_program::id()) - .data(vec![0], vec![0, 0]) - .verify(), - Ok(()), - "system program should be able to change account data size" - ); - } - - #[test] - fn test_verify_account_changes_owner_executable() { - let alice_program_id = solana_sdk::pubkey::new_rand(); - let bob_program_id = solana_sdk::pubkey::new_rand(); - - assert_eq!( - Change::new(&alice_program_id, &alice_program_id) - .owner(&bob_program_id) - .executable(false, true) - .verify(), - Err(InstructionError::ExecutableModified), - "program should not be able to change owner and executable at the same time" - ); - } -} diff --git a/programs/bpf_loader/benches/serialization.rs b/programs/bpf_loader/benches/serialization.rs index 87d1fc4f8a8c21..2acd8d374c1f8c 100644 --- a/programs/bpf_loader/benches/serialization.rs +++ b/programs/bpf_loader/benches/serialization.rs @@ -109,7 +109,7 @@ fn create_inputs(owner: Pubkey, num_instruction_accounts: usize) -> TransactionC } let mut transaction_context = - TransactionContext::new(transaction_accounts, Some(Rent::default()), 1, 1); + TransactionContext::new(transaction_accounts, Rent::default(), 1, 1); let instruction_data = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]; transaction_context .get_next_instruction_context() diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs index 0240ca65b0d54b..c2f481cf9f1383 100644 --- a/programs/bpf_loader/src/syscalls/cpi.rs +++ b/programs/bpf_loader/src/syscalls/cpi.rs @@ -1289,16 +1289,7 @@ fn update_callee_account( if !is_disable_cpi_setting_executable_and_rent_epoch_active && callee_account.borrow().rent_epoch() != caller_account.rent_epoch { - if invoke_context - .feature_set - .is_active(&enable_early_verification_of_account_modifications::id()) - { - return Err(Box::new(InstructionError::RentEpochModified)); - } else { - callee_account - .borrow_mut() - .set_rent_epoch(caller_account.rent_epoch); - } + return Err(Box::new(InstructionError::RentEpochModified)); } Ok(()) diff --git a/programs/bpf_loader/src/syscalls/mod.rs b/programs/bpf_loader/src/syscalls/mod.rs index c4a7fe1e6db50b..5afa861248df1a 100644 --- a/programs/bpf_loader/src/syscalls/mod.rs +++ b/programs/bpf_loader/src/syscalls/mod.rs @@ -37,8 +37,7 @@ use { self, blake3_syscall_enabled, curve25519_syscall_enabled, disable_cpi_setting_executable_and_rent_epoch, disable_deploy_of_alloc_free_syscall, disable_fees_sysvar, enable_alt_bn128_compression_syscall, enable_alt_bn128_syscall, - enable_big_mod_exp_syscall, enable_early_verification_of_account_modifications, - enable_partitioned_epoch_reward, enable_poseidon_syscall, + enable_big_mod_exp_syscall, enable_partitioned_epoch_reward, enable_poseidon_syscall, error_on_syscall_bpf_function_hash_collisions, last_restart_slot_sysvar, libsecp256k1_0_5_upgrade_enabled, reject_callx_r10, remaining_compute_units_syscall_enabled, stop_sibling_instruction_search_at_parent, diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index 871f4696c1a078..9df41327ab1a84 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -1236,7 +1236,7 @@ mod tests { // Create a fake TransactionContext with a fake InstructionContext with a single account which is the // vote account that was just created let transaction_context = - TransactionContext::new(vec![(node_pubkey, vote_account)], None, 0, 0); + TransactionContext::new(vec![(node_pubkey, vote_account)], Rent::default(), 0, 0); let mut instruction_context = InstructionContext::default(); instruction_context.configure( &[0], diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 68e5492186ff9d..0bba502b91a4b2 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -135,7 +135,6 @@ use { feature, feature_set::{ self, add_set_tx_loaded_accounts_data_size_instruction, - enable_early_verification_of_account_modifications, include_loaded_accounts_data_size_in_fee_calculation, remove_congestion_multiplier_from_fee_calculation, remove_deprecated_request_unit_ix, FeatureSet, @@ -4828,14 +4827,7 @@ impl Bank { let mut transaction_context = TransactionContext::new( transaction_accounts, - if self - .feature_set - .is_active(&enable_early_verification_of_account_modifications::id()) - { - Some(self.rent_collector.rent) - } else { - None - }, + self.rent_collector.rent, compute_budget.max_invoke_stack_height, if self .feature_set @@ -4885,7 +4877,6 @@ impl Bank { tx.message(), &loaded_transaction.program_indices, &mut transaction_context, - self.rent_collector.rent, log_collector.clone(), programs_loaded_for_tx_batch, &mut programs_modified_by_tx, @@ -4962,23 +4953,15 @@ impl Bank { { status = Err(TransactionError::UnbalancedTransaction); } - let mut accounts_data_len_delta = status - .as_ref() - .map_or(0, |info| info.accounts_data_len_delta); let status = status.map(|_| ()); loaded_transaction.accounts = accounts; - if self - .feature_set - .is_active(&enable_early_verification_of_account_modifications::id()) - { - saturating_add_assign!( - timings.details.total_account_count, - loaded_transaction.accounts.len() as u64 - ); - saturating_add_assign!(timings.details.changed_account_count, touched_account_count); - accounts_data_len_delta = status.as_ref().map_or(0, |_| accounts_resize_delta); - } + saturating_add_assign!( + timings.details.total_account_count, + loaded_transaction.accounts.len() as u64 + ); + saturating_add_assign!(timings.details.changed_account_count, touched_account_count); + let accounts_data_len_delta = status.as_ref().map_or(0, |_| accounts_resize_delta); let return_data = if enable_return_data_recording { if let Some(end_index) = return_data.data.iter().rposition(|&x| x != 0) { diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 58ce790d43d0d4..534169aca0e831 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -11475,7 +11475,7 @@ fn test_rent_state_list_len() { }); let transaction_context = TransactionContext::new( loaded_txs[0].0.as_ref().unwrap().accounts.clone(), - Some(Rent::default()), + Rent::default(), compute_budget.max_invoke_stack_height, compute_budget.max_instruction_trace_length, ); @@ -12165,7 +12165,6 @@ fn test_cap_accounts_data_allocations_per_transaction() { let (genesis_config, mint_keypair) = create_genesis_config(1_000_000 * LAMPORTS_PER_SOL); let mut bank = Bank::new_for_tests(&genesis_config); - bank.activate_feature(&feature_set::enable_early_verification_of_account_modifications::id()); bank.activate_feature(&feature_set::cap_accounts_data_allocations_per_transaction::id()); let mut instructions = Vec::new(); diff --git a/sdk/src/transaction_context.rs b/sdk/src/transaction_context.rs index cdfb162fc475a1..266456f219361d 100644 --- a/sdk/src/transaction_context.rs +++ b/sdk/src/transaction_context.rs @@ -60,19 +60,14 @@ pub type TransactionAccount = (Pubkey, AccountSharedData); pub struct TransactionAccounts { accounts: Vec>, touched_flags: RefCell>, - is_early_verification_of_account_modifications_enabled: bool, } impl TransactionAccounts { #[cfg(not(target_os = "solana"))] - fn new( - accounts: Vec>, - is_early_verification_of_account_modifications_enabled: bool, - ) -> TransactionAccounts { + fn new(accounts: Vec>) -> TransactionAccounts { TransactionAccounts { touched_flags: RefCell::new(vec![false; accounts.len()].into_boxed_slice()), accounts, - is_early_verification_of_account_modifications_enabled, } } @@ -86,13 +81,11 @@ impl TransactionAccounts { #[cfg(not(target_os = "solana"))] pub fn touch(&self, index: IndexOfAccount) -> Result<(), InstructionError> { - if self.is_early_verification_of_account_modifications_enabled { - *self - .touched_flags - .borrow_mut() - .get_mut(index as usize) - .ok_or(InstructionError::NotEnoughAccountKeys)? = true; - } + *self + .touched_flags + .borrow_mut() + .get_mut(index as usize) + .ok_or(InstructionError::NotEnoughAccountKeys)? = true; Ok(()) } @@ -150,7 +143,7 @@ pub struct TransactionContext { return_data: TransactionReturnData, accounts_resize_delta: RefCell, #[cfg(not(target_os = "solana"))] - rent: Option, + rent: Rent, #[cfg(not(target_os = "solana"))] is_cap_accounts_data_allocations_per_transaction_enabled: bool, /// Useful for debugging to filter by or to look it up on the explorer @@ -163,7 +156,7 @@ impl TransactionContext { #[cfg(not(target_os = "solana"))] pub fn new( transaction_accounts: Vec, - rent: Option, + rent: Rent, instruction_stack_capacity: usize, instruction_trace_capacity: usize, ) -> Self { @@ -173,7 +166,7 @@ impl TransactionContext { .unzip(); Self { account_keys: Pin::new(account_keys.into_boxed_slice()), - accounts: Rc::new(TransactionAccounts::new(accounts, rent.is_some())), + accounts: Rc::new(TransactionAccounts::new(accounts)), instruction_stack_capacity, instruction_trace_capacity, instruction_stack: Vec::with_capacity(instruction_stack_capacity), @@ -204,12 +197,6 @@ impl TransactionContext { &self.accounts } - /// Returns true if `enable_early_verification_of_account_modifications` is active - #[cfg(not(target_os = "solana"))] - pub fn is_early_verification_of_account_modifications_enabled(&self) -> bool { - self.rent.is_some() - } - /// Stores the signature of the current transaction #[cfg(all(not(target_os = "solana"), debug_assertions))] pub fn set_signature(&mut self, signature: &Signature) { @@ -342,9 +329,7 @@ impl TransactionContext { .ok_or(InstructionError::CallDepth)?; let callee_instruction_accounts_lamport_sum = self.instruction_accounts_lamport_sum(caller_instruction_context)?; - if !self.instruction_stack.is_empty() - && self.is_early_verification_of_account_modifications_enabled() - { + if !self.instruction_stack.is_empty() { let caller_instruction_context = self.get_current_instruction_context()?; let original_caller_instruction_accounts_lamport_sum = caller_instruction_context.instruction_accounts_lamport_sum; @@ -382,24 +367,20 @@ impl TransactionContext { } // Verify (before we pop) that the total sum of all lamports in this instruction did not change let detected_an_unbalanced_instruction = - if self.is_early_verification_of_account_modifications_enabled() { - self.get_current_instruction_context() - .and_then(|instruction_context| { - // Verify all executable accounts have no outstanding refs - for account_index in instruction_context.program_accounts.iter() { - self.get_account_at_index(*account_index)? - .try_borrow_mut() - .map_err(|_| InstructionError::AccountBorrowOutstanding)?; - } - self.instruction_accounts_lamport_sum(instruction_context) - .map(|instruction_accounts_lamport_sum| { - instruction_context.instruction_accounts_lamport_sum - != instruction_accounts_lamport_sum - }) - }) - } else { - Ok(false) - }; + self.get_current_instruction_context() + .and_then(|instruction_context| { + // Verify all executable accounts have no outstanding refs + for account_index in instruction_context.program_accounts.iter() { + self.get_account_at_index(*account_index)? + .try_borrow_mut() + .map_err(|_| InstructionError::AccountBorrowOutstanding)?; + } + self.instruction_accounts_lamport_sum(instruction_context) + .map(|instruction_accounts_lamport_sum| { + instruction_context.instruction_accounts_lamport_sum + != instruction_accounts_lamport_sum + }) + }); // Always pop, even if we `detected_an_unbalanced_instruction` self.instruction_stack.pop(); if detected_an_unbalanced_instruction? { @@ -430,9 +411,6 @@ impl TransactionContext { &self, instruction_context: &InstructionContext, ) -> Result { - if !self.is_early_verification_of_account_modifications_enabled() { - return Ok(0); - } let mut instruction_accounts_lamport_sum: u128 = 0; for instruction_account_index in 0..instruction_context.get_number_of_instruction_accounts() { @@ -771,32 +749,27 @@ impl<'a> BorrowedAccount<'a> { /// Assignes the owner of this account (transaction wide) #[cfg(not(target_os = "solana"))] pub fn set_owner(&mut self, pubkey: &[u8]) -> Result<(), InstructionError> { - if self - .transaction_context - .is_early_verification_of_account_modifications_enabled() - { - // Only the owner can assign a new owner - if !self.is_owned_by_current_program() { - return Err(InstructionError::ModifiedProgramId); - } - // and only if the account is writable - if !self.is_writable() { - return Err(InstructionError::ModifiedProgramId); - } - // and only if the account is not executable - if self.is_executable() { - return Err(InstructionError::ModifiedProgramId); - } - // and only if the data is zero-initialized or empty - if !is_zeroed(self.get_data()) { - return Err(InstructionError::ModifiedProgramId); - } - // don't touch the account if the owner does not change - if self.get_owner().to_bytes() == pubkey { - return Ok(()); - } - self.touch()?; + // Only the owner can assign a new owner + if !self.is_owned_by_current_program() { + return Err(InstructionError::ModifiedProgramId); + } + // and only if the account is writable + if !self.is_writable() { + return Err(InstructionError::ModifiedProgramId); + } + // and only if the account is not executable + if self.is_executable() { + return Err(InstructionError::ModifiedProgramId); + } + // and only if the data is zero-initialized or empty + if !is_zeroed(self.get_data()) { + return Err(InstructionError::ModifiedProgramId); } + // don't touch the account if the owner does not change + if self.get_owner().to_bytes() == pubkey { + return Ok(()); + } + self.touch()?; self.account.copy_into_owner_from_slice(pubkey); Ok(()) } @@ -810,28 +783,23 @@ impl<'a> BorrowedAccount<'a> { /// Overwrites the number of lamports of this account (transaction wide) #[cfg(not(target_os = "solana"))] pub fn set_lamports(&mut self, lamports: u64) -> Result<(), InstructionError> { - if self - .transaction_context - .is_early_verification_of_account_modifications_enabled() - { - // An account not owned by the program cannot have its balance decrease - if !self.is_owned_by_current_program() && lamports < self.get_lamports() { - return Err(InstructionError::ExternalAccountLamportSpend); - } - // The balance of read-only may not change - if !self.is_writable() { - return Err(InstructionError::ReadonlyLamportChange); - } - // The balance of executable accounts may not change - if self.is_executable() { - return Err(InstructionError::ExecutableLamportChange); - } - // don't touch the account if the lamports do not change - if self.get_lamports() == lamports { - return Ok(()); - } - self.touch()?; + // An account not owned by the program cannot have its balance decrease + if !self.is_owned_by_current_program() && lamports < self.get_lamports() { + return Err(InstructionError::ExternalAccountLamportSpend); + } + // The balance of read-only may not change + if !self.is_writable() { + return Err(InstructionError::ReadonlyLamportChange); } + // The balance of executable accounts may not change + if self.is_executable() { + return Err(InstructionError::ExecutableLamportChange); + } + // don't touch the account if the lamports do not change + if self.get_lamports() == lamports { + return Ok(()); + } + self.touch()?; self.account.set_lamports(lamports); Ok(()) } @@ -1034,7 +1002,6 @@ impl<'a> BorrowedAccount<'a> { pub fn is_rent_exempt_at_data_length(&self, data_length: usize) -> bool { self.transaction_context .rent - .unwrap_or_default() .is_exempt(self.get_lamports(), data_length) } @@ -1047,29 +1014,31 @@ impl<'a> BorrowedAccount<'a> { /// Configures whether this account is executable (transaction wide) #[cfg(not(target_os = "solana"))] pub fn set_executable(&mut self, is_executable: bool) -> Result<(), InstructionError> { - if let Some(rent) = self.transaction_context.rent { - // To become executable an account must be rent exempt - if !rent.is_exempt(self.get_lamports(), self.get_data().len()) { - return Err(InstructionError::ExecutableAccountNotRentExempt); - } - // Only the owner can set the executable flag - if !self.is_owned_by_current_program() { - return Err(InstructionError::ExecutableModified); - } - // and only if the account is writable - if !self.is_writable() { - return Err(InstructionError::ExecutableModified); - } - // one can not clear the executable flag - if self.is_executable() && !is_executable { - return Err(InstructionError::ExecutableModified); - } - // don't touch the account if the executable flag does not change - if self.is_executable() == is_executable { - return Ok(()); - } - self.touch()?; + // To become executable an account must be rent exempt + if !self + .transaction_context + .rent + .is_exempt(self.get_lamports(), self.get_data().len()) + { + return Err(InstructionError::ExecutableAccountNotRentExempt); + } + // Only the owner can set the executable flag + if !self.is_owned_by_current_program() { + return Err(InstructionError::ExecutableModified); } + // and only if the account is writable + if !self.is_writable() { + return Err(InstructionError::ExecutableModified); + } + // one can not clear the executable flag + if self.is_executable() && !is_executable { + return Err(InstructionError::ExecutableModified); + } + // don't touch the account if the executable flag does not change + if self.is_executable() == is_executable { + return Ok(()); + } + self.touch()?; self.account.set_executable(is_executable); Ok(()) } @@ -1118,12 +1087,6 @@ impl<'a> BorrowedAccount<'a> { /// Returns an error if the account data can not be mutated by the current program #[cfg(not(target_os = "solana"))] pub fn can_data_be_changed(&self) -> Result<(), InstructionError> { - if !self - .transaction_context - .is_early_verification_of_account_modifications_enabled() - { - return Ok(()); - } // Only non-executable accounts data can be changed if self.is_executable() { return Err(InstructionError::ExecutableDataModified); @@ -1142,12 +1105,6 @@ impl<'a> BorrowedAccount<'a> { /// Returns an error if the account data can not be resized to the given length #[cfg(not(target_os = "solana"))] pub fn can_data_be_resized(&self, new_length: usize) -> Result<(), InstructionError> { - if !self - .transaction_context - .is_early_verification_of_account_modifications_enabled() - { - return Ok(()); - } let old_length = self.get_data().len(); // Only the owner can change the length of the data if new_length != old_length && !self.is_owned_by_current_program() {