diff --git a/program-runtime/benches/pre_account.rs b/program-runtime/benches/pre_account.rs index 11569baf77..c0c7b19058 100644 --- a/program-runtime/benches/pre_account.rs +++ b/program-runtime/benches/pre_account.rs @@ -17,7 +17,7 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) { let non_owner = pubkey::new_rand(); let pre = PreAccount::new( &pubkey::new_rand(), - &AccountSharedData::new(0, BUFSIZE, &owner), + AccountSharedData::new(0, BUFSIZE, &owner), ); let post = AccountSharedData::new(0, BUFSIZE, &owner); assert_eq!( @@ -57,7 +57,7 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) { let pre = PreAccount::new( &pubkey::new_rand(), - &AccountSharedData::new(0, BUFSIZE, &owner), + AccountSharedData::new(0, BUFSIZE, &owner), ); bencher.iter(|| { pre.verify( diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 3285eddb7c..5b4aaac6f7 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -257,9 +257,9 @@ impl<'a> InvokeContext<'a> { self.pre_accounts = Vec::with_capacity(instruction.accounts.len()); let mut work = |_unique_index: usize, account_index: usize| { if account_index < self.accounts.len() { - let account = self.accounts[account_index].1.borrow(); + let account = self.accounts[account_index].1.borrow().clone(); self.pre_accounts - .push(PreAccount::new(&self.accounts[account_index].0, &account)); + .push(PreAccount::new(&self.accounts[account_index].0, account)); return Ok(()); } Err(InstructionError::MissingAccount) @@ -461,7 +461,7 @@ impl<'a> InvokeContext<'a> { .checked_add(u128::from(account.lamports())) .ok_or(InstructionError::UnbalancedInstruction)?; if is_writable && !pre_account.executable() { - pre_account.update(&account); + pre_account.update(account.clone()); } return Ok(()); } @@ -489,12 +489,12 @@ impl<'a> InvokeContext<'a> { let mut account_indices = Vec::with_capacity(message.account_keys.len()); let mut prev_account_sizes = Vec::with_capacity(message.account_keys.len()); for account_key in message.account_keys.iter() { - let (account_index, account) = self - .get_account(account_key) + let account_index = self + .find_index_of_account(account_key) .ok_or(InstructionError::MissingAccount)?; - let account_length = account.borrow().data().len(); + let account_length = self.accounts[account_index].1.borrow().data().len(); account_indices.push(account_index); - prev_account_sizes.push((account, account_length)); + prev_account_sizes.push((account_index, account_length)); } if let Some(instruction_recorder) = &self.instruction_recorder { @@ -510,8 +510,10 @@ impl<'a> InvokeContext<'a> { // Verify the called program has not misbehaved let do_support_realloc = self.feature_set.is_active(&do_support_realloc::id()); - for (account, prev_size) in prev_account_sizes.iter() { - if !do_support_realloc && *prev_size != account.borrow().data().len() && *prev_size != 0 + for (account_index, prev_size) in prev_account_sizes.into_iter() { + if !do_support_realloc + && prev_size != self.accounts[account_index].1.borrow().data().len() + && prev_size != 0 { // Only support for `CreateAccount` at this time. // Need a way to limit total realloc size across multiple CPI calls @@ -595,26 +597,27 @@ impl<'a> InvokeContext<'a> { // Find and validate executables / program accounts let callee_program_id = instruction.program_id; - let (program_account_index, program_account) = callee_keyed_accounts + let program_account_index = callee_keyed_accounts .iter() .find(|keyed_account| &callee_program_id == keyed_account.unsigned_key()) - .and_then(|_keyed_account| self.get_account(&callee_program_id)) + .and_then(|_keyed_account| self.find_index_of_account(&callee_program_id)) .ok_or_else(|| { ic_msg!(self, "Unknown program {}", callee_program_id); InstructionError::MissingAccount })?; - if !program_account.borrow().executable() { + let program_account = self.accounts[program_account_index].1.borrow(); + if !program_account.executable() { ic_msg!(self, "Account {} is not executable", callee_program_id); return Err(InstructionError::AccountNotExecutable); } let mut program_indices = vec![]; - if program_account.borrow().owner() == &bpf_loader_upgradeable::id() { + if program_account.owner() == &bpf_loader_upgradeable::id() { if let UpgradeableLoaderState::Program { programdata_address, - } = program_account.borrow().state()? + } = program_account.state()? { - if let Some((programdata_account_index, _programdata_account)) = - self.get_account(&programdata_address) + if let Some(programdata_account_index) = + self.find_index_of_account(&programdata_address) { program_indices.push(programdata_account_index); } else { @@ -797,16 +800,21 @@ impl<'a> InvokeContext<'a> { self.executors.borrow().get(pubkey) } - /// Find an account_index and account by its key - pub fn get_account(&self, pubkey: &Pubkey) -> Option<(usize, Rc>)> { - for (index, (key, account)) in self.accounts.iter().enumerate().rev() { + /// Finds an account_index by its key + pub fn find_index_of_account(&self, pubkey: &Pubkey) -> Option { + for (index, (key, _account)) in self.accounts.iter().enumerate().rev() { if key == pubkey { - return Some((index, account.clone())); + return Some(index); } } None } + /// Returns an account by its account_index + pub fn get_account_at_index(&self, account_index: usize) -> &RefCell { + &self.accounts[account_index].1 + } + /// Get this invocation's compute budget pub fn get_compute_budget(&self) -> &ComputeBudget { &self.current_compute_budget diff --git a/program-runtime/src/pre_account.rs b/program-runtime/src/pre_account.rs index 93fa2e88fe..a7240d8efb 100644 --- a/program-runtime/src/pre_account.rs +++ b/program-runtime/src/pre_account.rs @@ -8,11 +8,7 @@ use { system_instruction::MAX_PERMITTED_DATA_LENGTH, system_program, }, - std::{ - cell::{Ref, RefCell}, - fmt::Debug, - rc::Rc, - }, + std::fmt::Debug, }; // The relevant state of an account before an Instruction executes, used @@ -20,14 +16,14 @@ use { #[derive(Clone, Debug, Default)] pub struct PreAccount { key: Pubkey, - account: Rc>, + account: AccountSharedData, changed: bool, } impl PreAccount { - pub fn new(key: &Pubkey, account: &AccountSharedData) -> Self { + pub fn new(key: &Pubkey, account: AccountSharedData) -> Self { Self { key: *key, - account: Rc::new(RefCell::new(account.clone())), + account, changed: false, } } @@ -42,7 +38,7 @@ impl PreAccount { outermost_call: bool, do_support_realloc: bool, ) -> Result<(), InstructionError> { - let pre = self.account.borrow(); + let pre = &self.account; // Only the owner of the account may change owner and // only if the account is writable and @@ -157,11 +153,10 @@ impl PreAccount { Ok(()) } - pub fn update(&mut self, account: &AccountSharedData) { - let mut pre = self.account.borrow_mut(); - let rent_epoch = pre.rent_epoch(); - *pre = account.clone(); - pre.set_rent_epoch(rent_epoch); + 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; } @@ -170,16 +165,16 @@ impl PreAccount { &self.key } - pub fn data(&self) -> Ref<[u8]> { - Ref::map(self.account.borrow(), |account| account.data()) + pub fn data(&self) -> &[u8] { + self.account.data() } pub fn lamports(&self) -> u64 { - self.account.borrow().lamports() + self.account.lamports() } pub fn executable(&self) -> bool { - self.account.borrow().executable() + self.account.executable() } pub fn is_zeroed(buf: &[u8]) -> bool { @@ -236,10 +231,9 @@ mod tests { is_writable: true, pre: PreAccount::new( &solana_sdk::pubkey::new_rand(), - &AccountSharedData::from(Account { + AccountSharedData::from(Account { owner: *owner, lamports: std::u64::MAX, - data: vec![], ..Account::default() }), ), @@ -255,12 +249,12 @@ mod tests { self } pub fn executable(mut self, pre: bool, post: bool) -> Self { - self.pre.account.borrow_mut().set_executable(pre); + 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.borrow_mut().set_lamports(pre); + self.pre.account.set_lamports(pre); self.post.set_lamports(post); self } @@ -269,12 +263,12 @@ mod tests { self } pub fn data(mut self, pre: Vec, post: Vec) -> Self { - self.pre.account.borrow_mut().set_data(pre); + 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.borrow_mut().set_rent_epoch(pre); + self.pre.account.set_rent_epoch(pre); self.post.set_rent_epoch(post); self } diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index 4bd9e33a87..bbd1bd5f44 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -261,8 +261,8 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { let mut account_indices = Vec::with_capacity(message.account_keys.len()); let mut accounts = Vec::with_capacity(message.account_keys.len()); for (i, account_key) in message.account_keys.iter().enumerate() { - let ((account_index, account), account_info) = invoke_context - .get_account(account_key) + let (account_index, account_info) = invoke_context + .find_index_of_account(account_key) .zip( account_infos .iter() @@ -271,7 +271,9 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { .ok_or(InstructionError::MissingAccount) .unwrap(); { - let mut account = account.borrow_mut(); + let mut account = invoke_context + .get_account_at_index(account_index) + .borrow_mut(); account.copy_into_owner_from_slice(account_info.owner.as_ref()); account.set_data_from_slice(&account_info.try_borrow_data().unwrap()); account.set_lamports(account_info.lamports()); @@ -284,10 +286,9 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { None }; account_indices.push(account_index); - accounts.push((account, account_info)); + accounts.push((account_index, account_info)); } - let (program_account_index, _program_account) = - invoke_context.get_account(&program_id).unwrap(); + let program_account_index = invoke_context.find_index_of_account(&program_id).unwrap(); let program_indices = vec![program_account_index]; // Check Signers @@ -329,19 +330,20 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { .map_err(|err| ProgramError::try_from(err).unwrap_or_else(|err| panic!("{}", err)))?; // Copy writeable account modifications back into the caller's AccountInfos - for (account, account_info) in accounts.iter() { + for (account_index, account_info) in accounts.into_iter() { if let Some(account_info) = account_info { - **account_info.try_borrow_mut_lamports().unwrap() = account.borrow().lamports(); - let mut data = account_info.try_borrow_mut_data()?; + let account = invoke_context.get_account_at_index(account_index); let account_borrow = account.borrow(); + **account_info.try_borrow_mut_lamports().unwrap() = account_borrow.lamports(); + let mut data = account_info.try_borrow_mut_data()?; let new_data = account_borrow.data(); - if account_info.owner != account.borrow().owner() { + if account_info.owner != account_borrow.owner() { // TODO Figure out a better way to allow the System Program to set the account owner #[allow(clippy::transmute_ptr_to_ptr)] #[allow(mutable_transmutes)] let account_info_mut = unsafe { transmute::<&Pubkey, &mut Pubkey>(account_info.owner) }; - *account_info_mut = *account.borrow().owner(); + *account_info_mut = *account_borrow.owner(); } // TODO: Figure out how to allow the System Program to resize the account data assert!( diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index f1d97f858c..2f2c72409c 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -17,7 +17,7 @@ use { vm::{EbpfVm, SyscallObject, SyscallRegistry}, }, solana_sdk::{ - account::{AccountSharedData, ReadableAccount, WritableAccount}, + account::{ReadableAccount, WritableAccount}, account_info::AccountInfo, blake3, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, clock::Clock, @@ -1667,10 +1667,7 @@ struct CallerAccount<'a> { executable: bool, rent_epoch: u64, } -type TranslatedAccounts<'a> = ( - Vec, - Vec<(Rc>, Option>)>, -); +type TranslatedAccounts<'a> = (Vec, Vec<(usize, Option>)>); /// Implemented by language specific data structure translators trait SyscallInvokeSigned<'a, 'b> { @@ -2210,13 +2207,14 @@ where let mut account_indices = Vec::with_capacity(message.account_keys.len()); let mut accounts = Vec::with_capacity(message.account_keys.len()); for (i, account_key) in message.account_keys.iter().enumerate() { - if let Some((account_index, account)) = invoke_context.get_account(account_key) { + if let Some(account_index) = invoke_context.find_index_of_account(account_key) { + let account = invoke_context.get_account_at_index(account_index); if i == message.instructions[0].program_id_index as usize || account.borrow().executable() { // Use the known account account_indices.push(account_index); - accounts.push((account, None)); + accounts.push((account_index, None)); continue; } else if let Some(caller_account_index) = account_info_keys.iter().position(|key| *key == account_key) @@ -2265,7 +2263,7 @@ where None }; account_indices.push(account_index); - accounts.push((account, caller_account)); + accounts.push((account_index, caller_account)); continue; } } @@ -2400,9 +2398,11 @@ fn call<'a, 'b: 'a>( .map_err(SyscallError::InstructionError)?; // Copy results back to caller - for (callee_account, caller_account) in accounts.iter_mut() { + for (callee_account_index, caller_account) in accounts.iter_mut() { if let Some(caller_account) = caller_account { - let callee_account = callee_account.borrow(); + let callee_account = invoke_context + .get_account_at_index(*callee_account_index) + .borrow(); *caller_account.lamports = callee_account.lamports(); *caller_account.owner = *callee_account.owner(); let new_len = callee_account.data().len(); @@ -2672,7 +2672,9 @@ mod tests { solana_rbpf::{ ebpf::HOST_ALIGN, memory_region::MemoryRegion, user_error::UserError, vm::Config, }, - solana_sdk::{bpf_loader, fee_calculator::FeeCalculator, hash::hashv}, + solana_sdk::{ + account::AccountSharedData, bpf_loader, fee_calculator::FeeCalculator, hash::hashv, + }, std::str::FromStr, };