review feedback and fixing test

runtime/src/accounts/mod.rs

@@ -25,8 +25,7 @@ use {
     },
     solana_sdk::{
         account::{
-            is_builtin, is_builtin_or_executable, Account, AccountSharedData, ReadableAccount,
-            WritableAccount,
+            is_builtin, is_executable, Account, AccountSharedData, ReadableAccount, WritableAccount,
         },
         account_utils::StateMut,
         bpf_loader_upgradeable::{self, UpgradeableLoaderState},
@@ -307,7 +306,9 @@ fn load_transaction_accounts(
                             error_counters.invalid_program_for_execution += 1;
                             return Err(TransactionError::InvalidProgramForExecution);
                         }
-                    } else if is_builtin_or_executable(&account) && message.is_writable(i) {
+                    } else if (is_builtin(&account) || is_executable(&account))
+                        && message.is_writable(i)
+                    {
                         error_counters.invalid_writable_account += 1;
                         return Err(TransactionError::InvalidWritableAccount);
                     }
@@ -366,7 +367,7 @@ fn load_transaction_accounts(
                 return Err(TransactionError::ProgramAccountNotFound);
             }
 
-            if !is_builtin_or_executable(program_account) {
+            if !(is_builtin(program_account) || is_executable(program_account)) {
                 error_counters.invalid_program_for_execution += 1;
                 return Err(TransactionError::InvalidProgramForExecution);
             }
@@ -971,7 +972,7 @@ mod tests {
     }
 
     #[test]
-    fn test_load_accounts_not_executable() {
+    fn test_load_accounts_executable() {
         let mut accounts: Vec<TransactionAccount> = Vec::new();
         let mut error_counters = TransactionErrorMetrics::default();
 
@@ -980,9 +981,11 @@ mod tests {
         let key1 = Pubkey::from([5u8; 32]);
 
         let account = AccountSharedData::new(1, 0, &Pubkey::default());
+        assert!(!is_executable(&account));
         accounts.push((key0, account));
 
         let account = AccountSharedData::new(40, 1, &native_loader::id());
+        assert!(is_builtin(&account));
         accounts.push((key1, account));
 
         let instructions = vec![CompiledInstruction::new(1, &(), vec![0])];
@@ -996,12 +999,9 @@ mod tests {
 
         let loaded_accounts = load_accounts_aux_test(tx, &accounts, &mut error_counters);
 
-        assert_eq!(error_counters.invalid_program_for_execution, 1);
+        assert_eq!(error_counters.invalid_program_for_execution, 0);
         assert_eq!(loaded_accounts.len(), 1);
-        assert_eq!(
-            loaded_accounts[0],
-            (Err(TransactionError::InvalidProgramForExecution), None,)
-        );
+        assert!(loaded_accounts[0].0.is_ok());
     }
 
     #[test]

sdk/src/account.rs

@@ -764,19 +764,16 @@ pub const PROGRAM_OWNERS: &[Pubkey] = &[
     loader_v4::id(),
 ];
 
-/// Check if user program is executable.
+/// Return true if the account program is executable.
 pub fn is_executable(account: &AccountSharedData) -> bool {
     PROGRAM_OWNERS.iter().any(|v| account.owner() == v)
 }
 
+/// Return true if the account program is a builtin program.
 pub fn is_builtin(account: &AccountSharedData) -> bool {
     native_loader::check_id(account.owner())
 }
 
-pub fn is_builtin_or_executable(account: &AccountSharedData) -> bool {
-    is_builtin(account) || is_executable(account)
-}
-
 #[cfg(test)]
 pub mod tests {
     use super::*;

sdk/src/transaction_context.rs

@@ -17,7 +17,7 @@ use {
 };
 use {
     crate::{
-        account::{AccountSharedData, ReadableAccount},
+        account::{is_builtin, is_executable, AccountSharedData, ReadableAccount},
         instruction::InstructionError,
         pubkey::Pubkey,
     },
@@ -999,7 +999,7 @@ impl<'a> BorrowedAccount<'a> {
     /// Returns whether this account is executable (transaction wide)
     #[inline]
     pub fn is_executable(&self) -> bool {
-        self.account.executable()
+        is_builtin(&self.account) || is_executable(&self.account)
     }
 
     /// Configures whether this account is executable (transaction wide)