From 0d35872df117243c43ffec94c66a040bd20d4d84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Thu, 24 Jun 2021 12:33:38 +0200 Subject: [PATCH] Switch from using message.account_keys[i] to accounts[i].0 --- program-test/src/lib.rs | 9 ++---- programs/bpf_loader/src/syscalls.rs | 1 - rpc/src/rpc.rs | 26 ++++++++++------- runtime/src/accounts.rs | 10 ++----- runtime/src/bank.rs | 43 ++++++++++++----------------- runtime/src/message_processor.rs | 35 ++++++++++------------- 6 files changed, 53 insertions(+), 71 deletions(-) diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index fef1c5f1fa5d6d..3f365fb9b5eba6 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -336,16 +336,13 @@ 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 (i, account_pubkey) in message.account_keys.iter().enumerate() { + // REFACTOR: account_deps unification + for (i, (pubkey, account)) in accounts.iter().enumerate().take(message.account_keys.len()) { if !message.is_writable(i, true) { continue; } - for account_info in account_infos { - if account_info.unsigned_key() == account_pubkey { - let (_key, account) = &accounts[i]; - // REFACTOR: account_deps unification - assert_eq!(_key, account_pubkey); + if account_info.unsigned_key() == pubkey { **account_info.try_borrow_mut_lamports().unwrap() = account.borrow().lamports(); let mut data = account_info.try_borrow_mut_data()?; diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index b919e58e473c95..992e040b4e3445 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -2310,7 +2310,6 @@ fn call<'a>( let invoke_context = syscall.get_context()?; for (i, ((_key, account), account_ref)) in accounts.iter().zip(account_refs).enumerate() { // REFACTOR: account_deps unification - assert_eq!(message.account_keys[i], *_key); let account = account.borrow(); if let Some(mut account_ref) = account_ref { if message.is_writable(i, demote_sysvar_write_locks) && !account.executable() { diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 52bf7f2cfafea1..9251a1cfd8f34b 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -3090,16 +3090,22 @@ pub mod rpc_full { accounts.push(if result.is_err() { None } else { - transaction - .message - .account_keys - .iter() - .position(|pubkey| *pubkey == address) - .map(|i| post_simulation_accounts.get(i)) - .flatten() - .map(|(_pubkey, account)| { - // REFACTOR: account_deps unification - UiAccount::encode(&address, account, accounts_encoding, None, None) + // REFACTOR: account_deps unification + (0..transaction.message.account_keys.len()) + .position(|i| { + post_simulation_accounts + .get(i) + .map(|(key, _account)| *key == address) + .unwrap_or(false) + }) + .map(|i| { + UiAccount::encode( + &address, + &post_simulation_accounts[i].1, + accounts_encoding, + None, + None, + ) }) }); } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 1e863ff8eeaa1d..b59f27240fc4a5 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -993,15 +993,11 @@ impl Accounts { let message = &tx.message(); let loaded_transaction = raccs.as_mut().unwrap(); let mut fee_payer_index = None; - for ((i, key), (_key, account)) in message - .account_keys - .iter() - .enumerate() + for (i, (key, account)) in (0..message.account_keys.len()) .zip(loaded_transaction.accounts.iter_mut()) - .filter(|((i, key), (_key, _account))| message.is_non_loader_key(key, *i)) + .filter(|(i, (key, _account))| message.is_non_loader_key(key, *i)) { // REFACTOR: account_deps unification - assert_eq!(key, _key); let is_nonce_account = prepare_if_nonce_account( account, key, @@ -1038,7 +1034,7 @@ impl Accounts { .rent_debits .push(key, rent, account.lamports()); } - accounts.push((key, &*account)); + accounts.push((&*key, &*account)); } } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 635e4dc2418056..6fa73e89691ecd 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -631,30 +631,27 @@ impl NonceRollbackFull { nonce_address, nonce_account, } = partial; - let fee_payer = message - .account_keys - .iter() - .enumerate() - .find(|(i, k)| message.is_non_loader_key(k, *i)) - .and_then(|(i, k)| { - accounts.get(i).cloned().map(|(_k, a)| { - // REFACTOR: account_deps unification - assert_eq!(*k, _k); - (*k, a) - }) - }); + // REFACTOR: account_deps unification + let fee_payer = (0..message.account_keys.len()).find_map(|i| { + if let Some((k, a)) = &accounts.get(i) { + if message.is_non_loader_key(k, i) { + return Some((k, a)); + } + } + None + }); if let Some((fee_pubkey, fee_account)) = fee_payer { - if fee_pubkey == nonce_address { + if *fee_pubkey == nonce_address { Ok(Self { nonce_address, - nonce_account: fee_account, + nonce_account: fee_account.clone(), fee_account: None, }) } else { Ok(Self { nonce_address, nonce_account, - fee_account: Some(fee_account), + fee_account: Some(fee_account.clone()), }) } } else { @@ -2958,14 +2955,11 @@ impl Bank { TransactionAccountDepRefCells, TransactionLoaderRefCells, ) { - let account_refcells: Vec<_> = message - .account_keys - .iter() + let account_refcells: Vec<_> = (0..message.account_keys.len()) .zip(accounts.drain(..)) - .map(|(pubkey, (_pubkey, account))| { + .map(|(_i, (pubkey, account))| { // REFACTOR: account_deps unification - assert_eq!(*pubkey, _pubkey); - (*pubkey, Rc::new(RefCell::new(account))) + (pubkey, Rc::new(RefCell::new(account))) }) .collect(); let account_dep_refcells: Vec<_> = account_deps @@ -4821,14 +4815,11 @@ impl Bank { let message = &tx.message(); let loaded_transaction = raccs.as_ref().unwrap(); - for (pubkey, (_pubkey, account)) in message - .account_keys - .iter() + for (_i, (pubkey, account)) in (0..message.account_keys.len()) .zip(loaded_transaction.accounts.iter()) - .filter(|(_key, (_pubkey, account))| (Stakes::is_stake(account))) + .filter(|(_i, (_pubkey, account))| (Stakes::is_stake(account))) { // REFACTOR: account_deps unification - assert_eq!(pubkey, _pubkey); if Stakes::is_stake(account) { if let Some(old_vote_account) = self.stakes.write().unwrap().store( pubkey, diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index e2e45d32f694a7..1f3aa34a079f1c 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -471,14 +471,10 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> { self.feature_set.is_active(feature_id) } fn get_account(&self, pubkey: &Pubkey) -> Option>> { - if let Some(index) = self - .message - .account_keys - .iter() - .position(|key| key == pubkey) + if let Some(index) = + (0..self.message.account_keys.len()).position(|index| self.accounts[index].0 == *pubkey) { // REFACTOR: account_deps unification - assert_eq!(*pubkey, self.accounts[index].0); return Some(self.accounts[index].1.clone()); } self.account_deps.iter().find_map(|(key, account)| { @@ -640,11 +636,10 @@ impl MessageProcessor { .chain(instruction.accounts.iter().map(|index| { let index = *index as usize; // REFACTOR: account_deps unification - assert_eq!(message.account_keys[index], accounts[index].0); ( message.is_signer(index), message.is_writable(index, demote_sysvar_write_locks), - &message.account_keys[index], + &accounts[index].0, &accounts[index].1 as &RefCell, ) })) @@ -994,12 +989,13 @@ impl MessageProcessor { let mut pre_accounts = Vec::with_capacity(instruction.accounts.len()); { let mut work = |_unique_index: usize, account_index: usize| { - let key = &message.account_keys[account_index]; - let account = accounts[account_index].1.borrow(); - // REFACTOR: account_deps unification - assert_eq!(*key, accounts[account_index].0); - pre_accounts.push(PreAccount::new(key, &account)); - Ok(()) + if account_index < message.account_keys.len() && account_index < accounts.len() { + let account = accounts[account_index].1.borrow(); + // REFACTOR: account_deps unification + pre_accounts.push(PreAccount::new(&accounts[account_index].0, &account)); + return Ok(()); + } + Err(InstructionError::MissingAccount) }; let _ = instruction.visit_each_account(&mut work); } @@ -1096,10 +1092,8 @@ impl MessageProcessor { let (mut pre_sum, mut post_sum) = (0_u128, 0_u128); let mut work = |_unique_index: usize, account_index: usize| { if account_index < message.account_keys.len() && account_index < accounts.len() { - let key = &message.account_keys[account_index]; - let (_key, account) = &accounts[account_index]; + let (key, account) = &accounts[account_index]; // REFACTOR: account_deps unification - assert_eq!(key, _key); let is_writable = if let Some(caller_write_privileges) = caller_write_privileges { caller_write_privileges[account_index] } else { @@ -1169,11 +1163,10 @@ impl MessageProcessor { // Fixup the special instructions key if present // before the account pre-values are taken care of if feature_set.is_active(&instructions_sysvar_enabled::id()) { - for (i, key) in message.account_keys.iter().enumerate() { - if instructions::check_id(key) { + for (pubkey, accont) in accounts.iter().take(message.account_keys.len()) { + if instructions::check_id(pubkey) { // REFACTOR: account_deps unification - assert_eq!(*key, accounts[i].0); - let mut mut_account_ref = accounts[i].1.borrow_mut(); + let mut mut_account_ref = accont.borrow_mut(); instructions::store_current_index( mut_account_ref.data_as_mut_slice(), instruction_index as u16,