Cleanup - Feature gate of remove_accounts_executable_flag_checks (#6847)

Lichtso · web-flow · commit 3e1aa3befe0b · 2025-08-11T21:45:53.000+02:00
* Cleans the feature gate of remove_accounts_executable_flag_checks.

* Removes BorrowedAccount::is_executable_internal().
diff --git a/feature-set/src/lib.rs b/feature-set/src/lib.rs
@@ -104,8 +104,6 @@ impl FeatureSet {
         SVMFeatureSet {
             move_precompile_verification_to_svm: self
                 .is_active(&move_precompile_verification_to_svm::id()),
-            remove_accounts_executable_flag_checks: self
-                .is_active(&remove_accounts_executable_flag_checks::id()),
             stricter_abi_and_runtime_constraints: self
                 .is_active(&stricter_abi_and_runtime_constraints::id()),
             enable_bpf_loader_set_authority_checked_ix: self
diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs
@@ -444,15 +444,6 @@ impl<'a> InvokeContext<'a> {
                 })?;
             let borrowed_program_account = instruction_context
                 .try_borrow_instruction_account(self.transaction_context, program_account_index)?;
-            #[allow(deprecated)]
-            if !self
-                .get_feature_set()
-                .remove_accounts_executable_flag_checks
-                && !borrowed_program_account.is_executable()
-            {
-                ic_msg!(self, "Account {} is not executable", callee_program_id);
-                return Err(InstructionError::AccountNotExecutable);
-            }
 
             borrowed_program_account.get_index_in_transaction()
         };
@@ -561,21 +552,14 @@ impl<'a> InvokeContext<'a> {
             let owner_id = borrowed_root_account.get_owner();
             if native_loader::check_id(owner_id) {
                 *borrowed_root_account.get_key()
-            } else if self
-                .get_feature_set()
-                .remove_accounts_executable_flag_checks
+            } else if bpf_loader_deprecated::check_id(owner_id)
+                || bpf_loader::check_id(owner_id)
+                || bpf_loader_upgradeable::check_id(owner_id)
+                || loader_v4::check_id(owner_id)
             {
-                if bpf_loader_deprecated::check_id(owner_id)
-                    || bpf_loader::check_id(owner_id)
-                    || bpf_loader_upgradeable::check_id(owner_id)
-                    || loader_v4::check_id(owner_id)
-                {
-                    *owner_id
-                } else {
-                    return Err(InstructionError::UnsupportedProgramId);
-                }
-            } else {
                 *owner_id
+            } else {
+                return Err(InstructionError::UnsupportedProgramId);
             }
         };
 
diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs
@@ -403,46 +403,20 @@ pub(crate) fn process_instruction_inner(
             Err(InstructionError::UnsupportedProgramId)
         } else {
             ic_logger_msg!(log_collector, "Invalid BPF loader id");
-            Err(
-                if invoke_context
-                    .get_feature_set()
-                    .remove_accounts_executable_flag_checks
-                {
-                    InstructionError::UnsupportedProgramId
-                } else {
-                    InstructionError::IncorrectProgramId
-                },
-            )
+            Err(InstructionError::UnsupportedProgramId)
         }
         .map(|_| 0)
         .map_err(|error| Box::new(error) as Box<dyn std::error::Error>);
     }
 
     // Program Invocation
-    #[allow(deprecated)]
-    if !invoke_context
-        .get_feature_set()
-        .remove_accounts_executable_flag_checks
-        && !program_account.is_executable()
-    {
-        ic_logger_msg!(log_collector, "Program is not executable");
-        return Err(Box::new(InstructionError::IncorrectProgramId));
-    }
-
     let mut get_or_create_executor_time = Measure::start("get_or_create_executor_time");
     let executor = invoke_context
         .program_cache_for_tx_batch
         .find(program_account.get_key())
         .ok_or_else(|| {
             ic_logger_msg!(log_collector, "Program is not cached");
-            if invoke_context
-                .get_feature_set()
-                .remove_accounts_executable_flag_checks
-            {
-                InstructionError::UnsupportedProgramId
-            } else {
-                InstructionError::InvalidAccountData
-            }
+            InstructionError::UnsupportedProgramId
         })?;
     drop(program_account);
     get_or_create_executor_time.stop();
@@ -453,28 +427,10 @@ pub(crate) fn process_instruction_inner(
         | ProgramCacheEntryType::Closed
         | ProgramCacheEntryType::DelayVisibility => {
             ic_logger_msg!(log_collector, "Program is not deployed");
-            let instruction_error = if invoke_context
-                .get_feature_set()
-                .remove_accounts_executable_flag_checks
-            {
-                InstructionError::UnsupportedProgramId
-            } else {
-                InstructionError::InvalidAccountData
-            };
-            Err(Box::new(instruction_error) as Box<dyn std::error::Error>)
+            Err(Box::new(InstructionError::UnsupportedProgramId) as Box<dyn std::error::Error>)
         }
         ProgramCacheEntryType::Loaded(executable) => execute(executable, invoke_context),
-        _ => {
-            let instruction_error = if invoke_context
-                .get_feature_set()
-                .remove_accounts_executable_flag_checks
-            {
-                InstructionError::UnsupportedProgramId
-            } else {
-                InstructionError::IncorrectProgramId
-            };
-            Err(Box::new(instruction_error) as Box<dyn std::error::Error>)
-        }
+        _ => Err(Box::new(InstructionError::UnsupportedProgramId) as Box<dyn std::error::Error>),
     }
     .map(|_| 0)
 }
@@ -730,15 +686,6 @@ fn process_loader_upgradeable_instruction(
 
             let program =
                 instruction_context.try_borrow_instruction_account(transaction_context, 1)?;
-            #[allow(deprecated)]
-            if !invoke_context
-                .get_feature_set()
-                .remove_accounts_executable_flag_checks
-                && !program.is_executable()
-            {
-                ic_logger_msg!(log_collector, "Program account not executable");
-                return Err(InstructionError::AccountNotExecutable);
-            }
             if !program.is_writable() {
                 ic_logger_msg!(log_collector, "Program account not writeable");
                 return Err(InstructionError::InvalidArgument);
@@ -1881,13 +1828,11 @@ mod tests {
         solana_epoch_schedule::EpochSchedule,
         solana_instruction::{error::InstructionError, AccountMeta},
         solana_program_runtime::{
-            invoke_context::{mock_process_instruction, mock_process_instruction_with_feature_set},
-            with_mock_invoke_context,
+            invoke_context::mock_process_instruction, with_mock_invoke_context,
         },
         solana_pubkey::Pubkey,
         solana_rent::Rent,
         solana_sdk_ids::{system_program, sysvar},
-        solana_svm_feature_set::SVMFeatureSet,
         std::{fs::File, io::Read, ops::Range, sync::atomic::AtomicU64},
     };
 
@@ -2002,23 +1947,19 @@ mod tests {
             |_invoke_context| {},
         );
 
-        let mut feature_set = SVMFeatureSet::all_enabled();
-        feature_set.remove_accounts_executable_flag_checks = false;
-
         // Case: Account not a program
-        mock_process_instruction_with_feature_set(
+        mock_process_instruction(
             &loader_id,
             Some(0),
             &[],
             vec![(program_id, parameter_account.clone())],
             Vec::new(),
-            Err(InstructionError::IncorrectProgramId),
+            Err(InstructionError::UnsupportedProgramId),
             Entrypoint::vm,
             |invoke_context| {
                 test_utils::load_all_invoked_programs(invoke_context);
             },
             |_invoke_context| {},
-            &feature_set,
         );
         process_instruction(
             &loader_id,
@@ -2688,7 +2629,7 @@ mod tests {
             Err(InstructionError::AccountBorrowFailed),
         );
 
-        // Case: Program account not executable
+        // Case: Program account not a program
         let (transaction_accounts, mut instruction_accounts) = get_accounts(
             &buffer_address,
             &upgrade_authority_address,
@@ -2699,22 +2640,18 @@ mod tests {
         *instruction_accounts.get_mut(1).unwrap() = instruction_accounts.get(2).unwrap().clone();
         let instruction_data = bincode::serialize(&UpgradeableLoaderInstruction::Upgrade).unwrap();
 
-        let mut feature_set = SVMFeatureSet::all_enabled();
-        feature_set.remove_accounts_executable_flag_checks = false;
-
-        mock_process_instruction_with_feature_set(
+        mock_process_instruction(
             &bpf_loader_upgradeable::id(),
             None,
             &instruction_data,
             transaction_accounts.clone(),
             instruction_accounts.clone(),
-            Err(InstructionError::AccountNotExecutable),
+            Err(InstructionError::InvalidAccountData),
             Entrypoint::vm,
             |invoke_context| {
                 test_utils::load_all_invoked_programs(invoke_context);
             },
             |_invoke_context| {},
-            &feature_set,
         );
         process_instruction(
             transaction_accounts.clone(),
diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs
@@ -7209,7 +7209,6 @@ fn test_invoke_non_program_account_owned_by_a_builtin(
 ) {
     let (genesis_config, mint_keypair) = create_genesis_config(10000000);
     let mut bank = Bank::new_for_tests(&genesis_config);
-    bank.activate_feature(&feature_set::remove_accounts_executable_flag_checks::id());
     if formalize_loaded_transaction_data_size {
         bank.activate_feature(&feature_set::formalize_loaded_transaction_data_size::id());
     }
@@ -11551,11 +11550,7 @@ fn test_deploy_last_epoch_slot() {
         &mut genesis_config,
         agave_feature_set::enable_loader_v4::id(),
     );
-    genesis_config
-        .accounts
-        .remove(&feature_set::remove_accounts_executable_flag_checks::id());
-    let mut bank = Bank::new_for_tests(&genesis_config);
-    bank.activate_feature(&feature_set::remove_accounts_executable_flag_checks::id());
+    let bank = Bank::new_for_tests(&genesis_config);
 
     // go to the last slot in the epoch
     let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests();
@@ -11657,7 +11652,6 @@ fn test_loader_v3_to_v4_migration(formalize_loaded_transaction_data_size: bool)
         agave_feature_set::enable_loader_v4::id(),
     );
     let mut bank = Bank::new_for_tests(&genesis_config);
-    bank.activate_feature(&feature_set::remove_accounts_executable_flag_checks::id());
     if formalize_loaded_transaction_data_size {
         bank.activate_feature(&feature_set::formalize_loaded_transaction_data_size::id());
     }
diff --git a/svm-feature-set/src/lib.rs b/svm-feature-set/src/lib.rs
@@ -1,7 +1,6 @@
 #[derive(Clone, Copy, Default)]
 pub struct SVMFeatureSet {
     pub move_precompile_verification_to_svm: bool,
-    pub remove_accounts_executable_flag_checks: bool,
     pub stricter_abi_and_runtime_constraints: bool,
     pub enable_bpf_loader_set_authority_checked_ix: bool,
     pub enable_loader_v4: bool,
@@ -44,7 +43,6 @@ impl SVMFeatureSet {
     pub fn all_enabled() -> Self {
         Self {
             move_precompile_verification_to_svm: true,
-            remove_accounts_executable_flag_checks: true,
             stricter_abi_and_runtime_constraints: true,
             enable_bpf_loader_set_authority_checked_ix: true,
             enable_loader_v4: true,
diff --git a/svm/src/account_loader.rs b/svm/src/account_loader.rs
@@ -610,15 +610,6 @@ fn load_transaction_accounts_simd186<CB: TransactionProcessingCallback>(
             return Err(TransactionError::ProgramAccountNotFound);
         };
 
-        if !account_loader
-            .feature_set
-            .remove_accounts_executable_flag_checks
-            && !program_account.executable()
-        {
-            error_metrics.invalid_program_for_execution += 1;
-            return Err(TransactionError::InvalidProgramForExecution);
-        }
-
         let owner_id = program_account.owner();
         if !native_loader::check_id(owner_id) && !PROGRAM_OWNERS.contains(owner_id) {
             error_metrics.invalid_program_for_execution += 1;
@@ -690,28 +681,14 @@ fn load_transaction_accounts_old<CB: TransactionProcessingCallback>(
                 return Err(TransactionError::ProgramAccountNotFound);
             };
 
-            if !account_loader
-                .feature_set
-                .remove_accounts_executable_flag_checks
-                && !program_account.executable()
-            {
-                error_metrics.invalid_program_for_execution += 1;
-                return Err(TransactionError::InvalidProgramForExecution);
-            }
-
             let owner_id = program_account.owner();
             if native_loader::check_id(owner_id) {
                 return Ok(program_index as IndexOfAccount);
             }
 
             if !validated_loaders.contains(owner_id) {
                 if let Some(owner_account) = account_loader.load_account(owner_id) {
-                    if !native_loader::check_id(owner_account.owner())
-                        || (!account_loader
-                            .feature_set
-                            .remove_accounts_executable_flag_checks
-                            && !owner_account.executable())
-                    {
+                    if !native_loader::check_id(owner_account.owner()) {
                         error_metrics.invalid_program_for_execution += 1;
                         return Err(TransactionError::InvalidProgramForExecution);
                     }
@@ -1154,7 +1131,6 @@ mod tests {
         );
 
         let mut feature_set = SVMFeatureSet::all_enabled();
-        feature_set.remove_accounts_executable_flag_checks = false;
         feature_set.formalize_loaded_transaction_data_size = formalize_loaded_transaction_data_size;
 
         let load_results = load_accounts_with_features_and_rent(
@@ -1165,14 +1141,18 @@ mod tests {
             feature_set,
         );
 
-        assert_eq!(error_metrics.invalid_program_for_execution.0, 1);
-        assert!(matches!(
-            load_results,
-            TransactionLoadResult::FeesOnly(FeesOnlyTransaction {
-                load_error: TransactionError::InvalidProgramForExecution,
-                ..
-            }),
-        ));
+        assert_eq!(error_metrics.invalid_program_for_execution.0, 0);
+        match &load_results {
+            TransactionLoadResult::Loaded(loaded_transaction) => {
+                assert_eq!(loaded_transaction.accounts.len(), 2);
+                assert_eq!(loaded_transaction.accounts[0].1, accounts[0].1);
+                assert_eq!(loaded_transaction.accounts[1].1, accounts[1].1);
+                assert_eq!(loaded_transaction.program_indices.len(), 1);
+                assert_eq!(loaded_transaction.program_indices[0], 1);
+            }
+            TransactionLoadResult::FeesOnly(fees_only_tx) => panic!("{}", fees_only_tx.load_error),
+            TransactionLoadResult::NotLoaded(e) => panic!("{e}"),
+        }
     }
 
     #[test_case(false; "informal_loaded_size")]
@@ -2810,15 +2790,10 @@ mod tests {
 
     // note all magic numbers (how many accounts, how many instructions, how big to size buffers) are arbitrary
     // other than trying not to swamp programs with blank accounts and keep transaction size below the 64mb limit
-    #[test_case(false; "executable_mandatory")]
-    #[test_case(true; "executable_optional")]
-    fn test_load_transaction_accounts_data_sizes_simd186(
-        remove_accounts_executable_flag_checks: bool,
-    ) {
+    #[test]
+    fn test_load_transaction_accounts_data_sizes_simd186() {
         let mut rng = rand0_7::thread_rng();
         let mut mock_bank = TestCallbacks::default();
-        mock_bank.feature_set.remove_accounts_executable_flag_checks =
-            remove_accounts_executable_flag_checks;
 
         // arbitrary accounts
         for _ in 0..128 {
@@ -2857,7 +2832,7 @@ mod tests {
                     1,
                     vec![0; rng.gen_range(0, 512)],
                     *loader,
-                    !remove_accounts_executable_flag_checks || rng.gen(),
+                    rng.gen(),
                     u64::MAX,
                 );
 
@@ -2875,7 +2850,7 @@ mod tests {
                             1,
                             vec![0; rng.gen_range(0, 512)],
                             *loader,
-                            !remove_accounts_executable_flag_checks || rng.gen(),
+                            rng.gen(),
                             u64::MAX,
                         );
                         programdata_tracker.insert(
diff --git a/svm/src/transaction_processor.rs b/svm/src/transaction_processor.rs
@@ -850,11 +850,6 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
             compute_budget.max_instruction_stack_depth,
             compute_budget.max_instruction_trace_length,
         );
-        transaction_context.set_remove_accounts_executable_flag_checks(
-            environment
-                .feature_set
-                .remove_accounts_executable_flag_checks,
-        );
 
         let pre_account_state_info =
             TransactionAccountStateInfo::new(&transaction_context, tx, &environment.rent);
diff --git a/svm/tests/integration_test.rs b/svm/tests/integration_test.rs
diff --git a/transaction-context/src/lib.rs b/transaction-context/src/lib.rs
