Deprecate account meta executable read/update in bpf loaders

cli/src/program.rs

@@ -44,7 +44,7 @@ use {
     },
     solana_rpc_client_nonce_utils::blockhash_query::BlockhashQuery,
     solana_sdk::{
-        account::Account,
+        account::{is_executable, Account},
         account_utils::StateMut,
         bpf_loader, bpf_loader_deprecated,
         bpf_loader_upgradeable::{self, UpgradeableLoaderState},
@@ -1036,6 +1036,15 @@ fn get_default_program_keypair(program_location: &Option<String>) -> Keypair {
     program_keypair
 }
 
+fn is_account_executable(account: &Account) -> bool {
+    if account.owner == bpf_loader_deprecated::id() || account.owner == bpf_loader::id() {
+        account.executable
+    } else {
+        let feature_set = FeatureSet::all_enabled();
+        is_executable(account, &feature_set)
+    }
+}
+
 /// Deploy program using upgradeable loader. It also can process program upgrades
 #[allow(clippy::too_many_arguments)]
 fn process_program_deploy(
@@ -1092,7 +1101,7 @@ fn process_program_deploy(
             .into());
         }
 
-        if !account.executable {
+        if !is_account_executable(&account) {
             // Continue an initial deploy
             true
         } else if let Ok(UpgradeableLoaderState::Program {
@@ -2444,7 +2453,7 @@ fn complete_partial_program_init(
 ) -> Result<(Vec<Instruction>, u64), Box<dyn std::error::Error>> {
     let mut instructions: Vec<Instruction> = vec![];
     let mut balance_needed = 0;
-    if account.executable {
+    if is_account_executable(account) {
         return Err("Buffer account is already executable".into());
     }
     if account.owner != *loader_id && !system_program::check_id(&account.owner) {

cli/tests/program.rs

@@ -14,9 +14,11 @@ use {
     solana_rpc_client::rpc_client::RpcClient,
     solana_rpc_client_nonce_utils::blockhash_query::BlockhashQuery,
     solana_sdk::{
+        account::is_executable,
         account_utils::StateMut,
         bpf_loader_upgradeable::{self, UpgradeableLoaderState},
         commitment_config::CommitmentConfig,
+        feature_set::FeatureSet,
         pubkey::Pubkey,
         signature::{Keypair, NullSigner, Signer},
     },
@@ -100,7 +102,8 @@ fn test_cli_program_deploy_non_upgradeable() {
     let account0 = rpc_client.get_account(&program_id).unwrap();
     assert_eq!(account0.lamports, minimum_balance_for_program);
     assert_eq!(account0.owner, bpf_loader_upgradeable::id());
-    assert!(account0.executable);
+    assert!(is_executable(&account0, &FeatureSet::all_enabled()));
+
     let (programdata_pubkey, _) =
         Pubkey::find_program_address(&[program_id.as_ref()], &bpf_loader_upgradeable::id());
     let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap();
@@ -109,7 +112,10 @@ fn test_cli_program_deploy_non_upgradeable() {
         minimum_balance_for_programdata
     );
     assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
-    assert!(!programdata_account.executable);
+    assert!(!is_executable(
+        &programdata_account,
+        &FeatureSet::all_enabled()
+    ));
     assert_eq!(
         programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
         program_data[..]
@@ -137,7 +143,7 @@ fn test_cli_program_deploy_non_upgradeable() {
         .unwrap();
     assert_eq!(account1.lamports, minimum_balance_for_program);
     assert_eq!(account1.owner, bpf_loader_upgradeable::id());
-    assert!(account1.executable);
+    assert!(is_executable(&account1, &FeatureSet::all_enabled()));
     let (programdata_pubkey, _) = Pubkey::find_program_address(
         &[custom_address_keypair.pubkey().as_ref()],
         &bpf_loader_upgradeable::id(),
@@ -148,7 +154,10 @@ fn test_cli_program_deploy_non_upgradeable() {
         minimum_balance_for_programdata
     );
     assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
-    assert!(!programdata_account.executable);
+    assert!(!is_executable(
+        &programdata_account,
+        &FeatureSet::all_enabled()
+    ));
     assert_eq!(
         programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
         program_data[..]
@@ -376,7 +385,7 @@ fn test_cli_program_deploy_with_authority() {
     let program_account = rpc_client.get_account(&program_keypair.pubkey()).unwrap();
     assert_eq!(program_account.lamports, minimum_balance_for_program);
     assert_eq!(program_account.owner, bpf_loader_upgradeable::id());
-    assert!(program_account.executable);
+    assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
     let (programdata_pubkey, _) = Pubkey::find_program_address(
         &[program_keypair.pubkey().as_ref()],
         &bpf_loader_upgradeable::id(),
@@ -387,7 +396,10 @@ fn test_cli_program_deploy_with_authority() {
         minimum_balance_for_programdata
     );
     assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
-    assert!(!programdata_account.executable);
+    assert!(!is_executable(
+        &programdata_account,
+        &FeatureSet::all_enabled()
+    ));
     assert_eq!(
         programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
         program_data[..]
@@ -421,7 +433,7 @@ fn test_cli_program_deploy_with_authority() {
     let program_account = rpc_client.get_account(&program_pubkey).unwrap();
     assert_eq!(program_account.lamports, minimum_balance_for_program);
     assert_eq!(program_account.owner, bpf_loader_upgradeable::id());
-    assert!(program_account.executable);
+    assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
     let (programdata_pubkey, _) =
         Pubkey::find_program_address(&[program_pubkey.as_ref()], &bpf_loader_upgradeable::id());
     let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap();
@@ -430,7 +442,10 @@ fn test_cli_program_deploy_with_authority() {
         minimum_balance_for_programdata
     );
     assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
-    assert!(!programdata_account.executable);
+    assert!(!is_executable(
+        &programdata_account,
+        &FeatureSet::all_enabled()
+    ));
     assert_eq!(
         programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
         program_data[..]
@@ -455,7 +470,7 @@ fn test_cli_program_deploy_with_authority() {
     let program_account = rpc_client.get_account(&program_pubkey).unwrap();
     assert_eq!(program_account.lamports, minimum_balance_for_program);
     assert_eq!(program_account.owner, bpf_loader_upgradeable::id());
-    assert!(program_account.executable);
+    assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
     let (programdata_pubkey, _) =
         Pubkey::find_program_address(&[program_pubkey.as_ref()], &bpf_loader_upgradeable::id());
     let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap();
@@ -464,7 +479,10 @@ fn test_cli_program_deploy_with_authority() {
         minimum_balance_for_programdata
     );
     assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
-    assert!(!programdata_account.executable);
+    assert!(!is_executable(
+        &programdata_account,
+        &FeatureSet::all_enabled()
+    ));
     assert_eq!(
         programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
         program_data[..]
@@ -511,7 +529,7 @@ fn test_cli_program_deploy_with_authority() {
     let program_account = rpc_client.get_account(&program_pubkey).unwrap();
     assert_eq!(program_account.lamports, minimum_balance_for_program);
     assert_eq!(program_account.owner, bpf_loader_upgradeable::id());
-    assert!(program_account.executable);
+    assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
     let (programdata_pubkey, _) =
         Pubkey::find_program_address(&[program_pubkey.as_ref()], &bpf_loader_upgradeable::id());
     let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap();
@@ -520,7 +538,10 @@ fn test_cli_program_deploy_with_authority() {
         minimum_balance_for_programdata
     );
     assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
-    assert!(!programdata_account.executable);
+    assert!(!is_executable(
+        &programdata_account,
+        &FeatureSet::all_enabled()
+    ));
     assert_eq!(
         programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
         program_data[..]

program-runtime/src/invoke_context.rs

@@ -962,8 +962,8 @@ mod tests {
         let owned_account = AccountSharedData::new(42, 1, &callee_program_id);
         let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand());
         let readonly_account = AccountSharedData::new(168, 1, &solana_sdk::pubkey::new_rand());
-        let loader_account = AccountSharedData::new(0, 0, &native_loader::id());
-        let mut program_account = AccountSharedData::new(1, 0, &native_loader::id());
+        let loader_account = AccountSharedData::new(0, 1, &native_loader::id());
+        let mut program_account = AccountSharedData::new(1, 1, &native_loader::id());
         program_account.set_executable(true);
         let transaction_accounts = vec![
             (solana_sdk::pubkey::new_rand(), owned_account),
@@ -990,7 +990,7 @@ mod tests {
         let mut programs_loaded_for_tx_batch = LoadedProgramsForTxBatch::default();
         programs_loaded_for_tx_batch.replenish(
             callee_program_id,
-            Arc::new(LoadedProgram::new_builtin(0, 0, MockBuiltin::vm)),
+            Arc::new(LoadedProgram::new_builtin(0, 1, MockBuiltin::vm)),
         );
         invoke_context.programs_loaded_for_tx_batch = &programs_loaded_for_tx_batch;
 

programs/bpf_loader/src/lib.rs

@@ -34,7 +34,8 @@ use {
         clock::Slot,
         entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
         feature_set::{
-            bpf_account_data_direct_mapping, enable_bpf_loader_extend_program_ix,
+            bpf_account_data_direct_mapping, deprecate_executable_meta_update_in_bpf_loader,
+            disable_bpf_loader_instructions, enable_bpf_loader_extend_program_ix,
             enable_bpf_loader_set_authority_checked_ix, native_programs_consume_cu,
             remove_bpf_loader_incorrect_program_id, FeatureSet,
         },
@@ -785,7 +786,15 @@ fn process_loader_upgradeable_instruction(
                 },
                 &invoke_context.feature_set,
             )?;
-            program.set_executable(true)?;
+
+            // Skip writing true to executable meta after bpf program deployment when
+            // `deprecate_executable_meta_update_in_bpf_loader` feature is activated.
+            if !invoke_context
+                .feature_set
+                .is_active(&deprecate_executable_meta_update_in_bpf_loader::id())
+            {
+                program.set_executable(true)?;
+            }
             drop(program);
 
             ic_logger_msg!(log_collector, "Deployed program {:?}", new_program_id);
@@ -1469,6 +1478,20 @@ fn process_loader_instruction(invoke_context: &mut InvokeContext) -> Result<(),
         );
         return Err(InstructionError::IncorrectProgramId);
     }
+
+    // Return `UnsupportedProgramId` error for bpf_loader when
+    // `disable_bpf_loader_instruction` feature is activated.
+    if invoke_context
+        .feature_set
+        .is_active(&disable_bpf_loader_instructions::id())
+    {
+        ic_msg!(
+            invoke_context,
+            "BPF loader management instructions are no longer supported"
+        );
+        return Err(InstructionError::UnsupportedProgramId);
+    }
+
     let is_program_signer = program.is_signer();
     match limited_deserialize(instruction_data)? {
         LoaderInstruction::Write { offset, bytes } => {
@@ -1493,6 +1516,13 @@ fn process_loader_instruction(invoke_context: &mut InvokeContext) -> Result<(),
                 {},
                 program.get_data(),
             );
+
+            // `deprecate_executable_meta_update_in_bpf_loader` feature doesn't
+            // apply to  bpf_loader v2. Instead, the deployment by bpf_loader
+            // will be deprecated by its own feature
+            // `disable_bpf_loader_instructions`. Before we activate
+            // deprecate_executable_meta_update_in_bpf_loader, we should
+            // activate `disable_bpf_loader_instructions` first.
             program.set_executable(true)?;
             ic_msg!(invoke_context, "Finalized account {:?}", program.get_key());
         }
@@ -1779,6 +1809,10 @@ mod tests {
             expected_result,
             Entrypoint::vm,
             |invoke_context| {
+                let mut features = FeatureSet::all_enabled();
+                features.deactivate(&disable_bpf_loader_instructions::id());
+                features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
+                invoke_context.feature_set = Arc::new(features);
                 test_utils::load_all_invoked_programs(invoke_context);
             },
             |_invoke_context| {},
@@ -1998,6 +2032,10 @@ mod tests {
             Err(InstructionError::ProgramFailedToComplete),
             Entrypoint::vm,
             |invoke_context| {
+                let mut features = FeatureSet::all_enabled();
+                features.deactivate(&disable_bpf_loader_instructions::id());
+                features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
+                invoke_context.feature_set = Arc::new(features);
                 invoke_context.mock_set_remaining(0);
                 test_utils::load_all_invoked_programs(invoke_context);
             },
@@ -2543,7 +2581,12 @@ mod tests {
                 instruction_accounts,
                 expected_result,
                 Entrypoint::vm,
-                |_invoke_context| {},
+                |invoke_context| {
+                    let mut features = FeatureSet::all_enabled();
+                    features.deactivate(&disable_bpf_loader_instructions::id());
+                    features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
+                    invoke_context.feature_set = Arc::new(features);
+                },
                 |_invoke_context| {},
             )
         }

programs/bpf_loader/src/serialization.rs

@@ -938,7 +938,7 @@ mod tests {
                 assert_eq!(account.lamports(), account_info.lamports());
                 assert_eq!(account.data(), &account_info.data.borrow()[..]);
                 assert_eq!(account.owner(), account_info.owner);
-                assert_eq!(account.executable(), account_info.executable);
+                assert!(account_info.executable);
                 assert_eq!(account.rent_epoch(), account_info.rent_epoch);
 
                 assert_eq!(
@@ -1023,7 +1023,7 @@ mod tests {
                 assert_eq!(account.lamports(), account_info.lamports());
                 assert_eq!(account.data(), &account_info.data.borrow()[..]);
                 assert_eq!(account.owner(), account_info.owner);
-                assert_eq!(account.executable(), account_info.executable);
+                assert!(account_info.executable);
                 assert_eq!(account.rent_epoch(), account_info.rent_epoch);
             }
 

programs/loader-v4/src/lib.rs

@@ -466,6 +466,7 @@ pub fn process_instruction_retract(
     let transaction_context = &invoke_context.transaction_context;
     let instruction_context = transaction_context.get_current_instruction_context()?;
     let mut program = instruction_context.try_borrow_instruction_account(transaction_context, 0)?;
+
     let authority_address = instruction_context
         .get_index_of_instruction_account_in_transaction(1)
         .and_then(|index| transaction_context.get_key_of_account_at_index(index))?;

programs/sbf/benches/bpf_loader.rs

@@ -34,7 +34,7 @@ use {
         bpf_loader,
         client::SyncClient,
         entrypoint::SUCCESS,
-        feature_set::FeatureSet,
+        feature_set::{self, FeatureSet},
         instruction::{AccountMeta, Instruction},
         message::Message,
         native_loader,
@@ -185,10 +185,21 @@ fn bench_program_alu(bencher: &mut Bencher) {
 #[bench]
 fn bench_program_execute_noop(bencher: &mut Bencher) {
     let GenesisConfigInfo {
-        genesis_config,
+        mut genesis_config,
         mint_keypair,
         ..
     } = create_genesis_config(50);
+
+    // deactivate `disable_bpf_loader_instructions` feature so that the program
+    // can be loaded, finalized and benched.
+    genesis_config
+        .accounts
+        .remove(&feature_set::disable_bpf_loader_instructions::id());
+
+    genesis_config
+        .accounts
+        .remove(&feature_set::deprecate_executable_meta_update_in_bpf_loader::id());
+
     let bank = Bank::new_for_benches(&genesis_config);
     let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests();
     let mut bank_client = BankClient::new_shared(bank.clone());