From e806d312241cedb6a6e060bfc3481bf93b4d7656 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Mon, 12 Jul 2021 10:42:22 -0500 Subject: [PATCH] Optimize Message::is_non_loader_key method (#18579) --- cli-output/src/display.rs | 2 +- runtime/src/accounts.rs | 7 ++-- runtime/src/bank.rs | 2 +- sdk/program/src/message.rs | 65 ++++++++++++-------------------------- 4 files changed, 25 insertions(+), 51 deletions(-) diff --git a/cli-output/src/display.rs b/cli-output/src/display.rs index f8f7e8274..3d1c3e74f 100644 --- a/cli-output/src/display.rs +++ b/cli-output/src/display.rs @@ -201,7 +201,7 @@ pub fn write_transaction( } let mut fee_payer_index = None; for (account_index, account) in message.account_keys.iter().enumerate() { - if fee_payer_index.is_none() && message.is_non_loader_key(account, account_index) { + if fee_payer_index.is_none() && message.is_non_loader_key(account_index) { fee_payer_index = Some(account_index) } writeln!( diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index eafa3fd9c..66792a1ce 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -28,7 +28,7 @@ use solana_sdk::{ fee_calculator::FeeCalculator, genesis_config::ClusterType, hash::Hash, - message::{Message, MessageProgramIdsCache}, + message::Message, native_loader, nonce, pubkey::Pubkey, transaction::Result, @@ -208,12 +208,11 @@ impl Accounts { let mut tx_rent: TransactionRent = 0; let mut accounts = Vec::with_capacity(message.account_keys.len()); let mut account_deps = Vec::with_capacity(message.account_keys.len()); - let mut key_check = MessageProgramIdsCache::new(message); let mut rent_debits = RentDebits::default(); let rent_for_sysvars = feature_set.is_active(&feature_set::rent_for_sysvars::id()); for (i, key) in message.account_keys.iter().enumerate() { - let account = if key_check.is_non_loader_key(key, i) { + let account = if message.is_non_loader_key(i) { if payer_index.is_none() { payer_index = Some(i); } @@ -981,7 +980,7 @@ impl Accounts { let mut fee_payer_index = None; for (i, (key, account)) in (0..message.account_keys.len()) .zip(loaded_transaction.accounts.iter_mut()) - .filter(|(i, (key, _account))| message.is_non_loader_key(key, *i)) + .filter(|(i, _account)| message.is_non_loader_key(*i)) { let is_nonce_account = prepare_if_nonce_account( account, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 8cb3149ec..ff93f1256 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -632,7 +632,7 @@ impl NonceRollbackFull { } = partial; 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) { + if message.is_non_loader_key(i) { return Some((k, a)); } } diff --git a/sdk/program/src/message.rs b/sdk/program/src/message.rs index 28b44eec3..67e360f9d 100644 --- a/sdk/program/src/message.rs +++ b/sdk/program/src/message.rs @@ -240,26 +240,6 @@ impl Sanitize for Message { Ok(()) } } -pub struct MessageProgramIdsCache<'a> { - program_ids: Option>, - message: &'a Message, -} - -impl<'a> MessageProgramIdsCache<'a> { - pub fn new(message: &'a Message) -> Self { - Self { - program_ids: None, - message, - } - } - pub fn is_non_loader_key(&mut self, key: &Pubkey, key_index: usize) -> bool { - if self.program_ids.is_none() { - self.program_ids = Some(self.message.program_ids()); - } - self.message - .is_non_loader_key_internal(self.program_ids.as_ref().unwrap(), key, key_index) - } -} impl Message { pub fn new_with_compiled_instructions( @@ -356,28 +336,28 @@ impl Message { .collect() } - pub fn is_key_passed_to_program(&self, index: usize) -> bool { - if let Ok(index) = u8::try_from(index) { - for ix in self.instructions.iter() { - if ix.accounts.contains(&index) { - return true; - } - } + pub fn is_key_passed_to_program(&self, key_index: usize) -> bool { + if let Ok(key_index) = u8::try_from(key_index) { + self.instructions + .iter() + .any(|ix| ix.accounts.contains(&key_index)) + } else { + false } - false } - pub(crate) fn is_non_loader_key_internal( - &self, - program_ids: &[&Pubkey], - key: &Pubkey, - key_index: usize, - ) -> bool { - !program_ids.contains(&key) || self.is_key_passed_to_program(key_index) + pub fn is_key_called_as_program(&self, key_index: usize) -> bool { + if let Ok(key_index) = u8::try_from(key_index) { + self.instructions + .iter() + .any(|ix| ix.program_id_index == key_index) + } else { + false + } } - pub fn is_non_loader_key(&self, key: &Pubkey, key_index: usize) -> bool { - self.is_non_loader_key_internal(&self.program_ids(), key, key_index) + pub fn is_non_loader_key(&self, key_index: usize) -> bool { + !self.is_key_called_as_program(key_index) || self.is_key_passed_to_program(key_index) } pub fn program_position(&self, index: usize) -> Option { @@ -1029,14 +1009,9 @@ mod tests { Hash::default(), instructions, ); - let mut helper = MessageProgramIdsCache::new(&message); - - assert!(message.is_non_loader_key(&key0, 0)); - assert!(message.is_non_loader_key(&key1, 1)); - assert!(!message.is_non_loader_key(&loader2, 2)); - assert!(helper.is_non_loader_key(&key0, 0)); - assert!(helper.is_non_loader_key(&key1, 1)); - assert!(!helper.is_non_loader_key(&loader2, 2)); + assert!(message.is_non_loader_key(0)); + assert!(message.is_non_loader_key(1)); + assert!(!message.is_non_loader_key(2)); } #[test]