Don't require program_id to passed down cpi call chain

program-runtime/src/invoke_context.rs

@@ -18,9 +18,11 @@ use {
         vm::{Config, ContextObject, EbpfVm},
     },
     solana_sdk::{
-        account::AccountSharedData,
+        account::{AccountSharedData, ReadableAccount},
         bpf_loader_deprecated,
-        feature_set::{native_programs_consume_cu, FeatureSet},
+        feature_set::{
+            native_programs_consume_cu, no_need_for_program_key_in_parent_instruction, FeatureSet,
+        },
         hash::Hash,
         instruction::{AccountMeta, InstructionError},
         native_loader,
@@ -402,23 +404,50 @@ impl<'a> InvokeContext<'a> {
 
         // Find and validate executables / program accounts
         let callee_program_id = instruction.program_id;
-        let program_account_index = instruction_context
-            .find_index_of_instruction_account(self.transaction_context, &callee_program_id)
-            .ok_or_else(|| {
+
+        if self
+            .feature_set
+            .is_active(&no_need_for_program_key_in_parent_instruction::id())
+        {
+            let Some(program_account_index) = self
+                .transaction_context
+                .find_index_of_program_account(&callee_program_id)
+            else {
                 ic_msg!(self, "Unknown program {}", callee_program_id);
-                InstructionError::MissingAccount
-            })?;
-        let borrowed_program_account = instruction_context
-            .try_borrow_instruction_account(self.transaction_context, program_account_index)?;
-        if !borrowed_program_account.is_executable() {
-            ic_msg!(self, "Account {} is not executable", callee_program_id);
-            return Err(InstructionError::AccountNotExecutable);
-        }
+                return Err(InstructionError::MissingAccount);
+            };
+
+            let borrowed_program_account = self
+                .transaction_context
+                .accounts()
+                .try_borrow(program_account_index)?;
 
-        Ok((
-            instruction_accounts,
-            vec![borrowed_program_account.get_index_in_transaction()],
-        ))
+            if !borrowed_program_account.executable() {
+                ic_msg!(self, "Account {} is not executable", callee_program_id);
+                return Err(InstructionError::AccountNotExecutable);
+            }
+
+            Ok((instruction_accounts, vec![program_account_index]))
+        } else {
+            let Some(program_account_index) = instruction_context
+                .find_index_of_instruction_account(self.transaction_context, &callee_program_id)
+            else {
+                ic_msg!(self, "Unknown program {}", callee_program_id);
+                return Err(InstructionError::MissingAccount);
+            };
+
+            let borrowed_program_account = instruction_context
+                .try_borrow_instruction_account(self.transaction_context, program_account_index)?;
+            if !borrowed_program_account.is_executable() {
+                ic_msg!(self, "Account {} is not executable", callee_program_id);
+                return Err(InstructionError::AccountNotExecutable);
+            }
+
+            Ok((
+                instruction_accounts,
+                vec![borrowed_program_account.get_index_in_transaction()],
+            ))
+        }
     }
 
     /// Processes an instruction and returns how many compute units were used

programs/sbf/tests/programs.rs

@@ -43,7 +43,10 @@ use {
         clock::MAX_PROCESSING_AGE,
         compute_budget::ComputeBudgetInstruction,
         entrypoint::MAX_PERMITTED_DATA_INCREASE,
-        feature_set::{self, remove_deprecated_request_unit_ix, FeatureSet},
+        feature_set::{
+            self, no_need_for_program_key_in_parent_instruction, remove_deprecated_request_unit_ix,
+            FeatureSet,
+        },
         fee::FeeStructure,
         loader_instruction,
         message::{v0::LoadedAddresses, SanitizedMessage},
@@ -1232,6 +1235,85 @@ fn test_program_sbf_caller_has_access_to_cpi_program() {
         ..
     } = create_genesis_config(50);
     let bank = Bank::new_for_tests(&genesis_config);
+
+    let bank = Arc::new(bank);
+    let mut bank_client = BankClient::new_shared(bank.clone());
+
+    let caller_pubkey = load_program(
+        &bank_client,
+        &bpf_loader::id(),
+        &mint_keypair,
+        "solana_sbf_rust_caller_access",
+    );
+    let (_, caller2_pubkey) = load_program_and_advance_slot(
+        &mut bank_client,
+        &bpf_loader::id(),
+        &mint_keypair,
+        "solana_sbf_rust_caller_access",
+    );
+
+    // First try it with all the accounts
+    let account_metas = vec![
+        AccountMeta::new_readonly(caller_pubkey, false),
+        AccountMeta::new_readonly(caller2_pubkey, false),
+    ];
+    let instruction = Instruction::new_with_bytes(caller_pubkey, &[1], account_metas.clone());
+    let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
+    assert!(result.is_ok(), "{result:?}");
+
+    // Trying cpi into account that does not exist
+    let random_pubkey = Pubkey::new_unique();
+    let account_metas = vec![
+        AccountMeta::new_readonly(caller_pubkey, false),
+        AccountMeta::new_readonly(random_pubkey, false),
+    ];
+    let instruction = Instruction::new_with_bytes(caller_pubkey, &[1], account_metas);
+    let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
+    assert_eq!(
+        result.unwrap_err().unwrap(),
+        TransactionError::InstructionError(0, InstructionError::AccountNotExecutable)
+    );
+
+    // Trying cpi into account that is not part of the message at all
+    let account_metas = vec![AccountMeta::new_readonly(caller_pubkey, false)];
+
+    let instruction = Instruction::new_with_bytes(
+        caller_pubkey,
+        caller2_pubkey.to_bytes().as_ref(),
+        account_metas.clone(),
+    );
+    let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
+    assert_eq!(
+        result.unwrap_err().unwrap(),
+        TransactionError::InstructionError(0, InstructionError::MissingAccount)
+    );
+
+    // Now add it to the message accounts - CPI without caller2 in the meta
+    let instruction = Instruction::new_with_bytes(
+        caller_pubkey,
+        caller2_pubkey.to_bytes().as_ref(),
+        account_metas,
+    );
+    let message = Message::new_with_programs(
+        &[instruction],
+        &[caller2_pubkey],
+        Some(&mint_keypair.pubkey()),
+    );
+    let result = bank_client.send_and_confirm_message(&[&mint_keypair], message);
+    assert!(result.is_ok(), "{result:?}");
+}
+
+#[test]
+#[cfg(feature = "sbf_rust")]
+fn test_program_sbf_caller_has_access_to_cpi_program_before() {
+    let GenesisConfigInfo {
+        genesis_config,
+        mint_keypair,
+        ..
+    } = create_genesis_config(50);
+    let mut bank = Bank::new_for_tests(&genesis_config);
+    bank.deactivate_feature(&no_need_for_program_key_in_parent_instruction::id());
+
     let bank = Arc::new(bank);
     let mut bank_client = BankClient::new_shared(bank.clone());
 
@@ -1247,6 +1329,8 @@ fn test_program_sbf_caller_has_access_to_cpi_program() {
         &mint_keypair,
         "solana_sbf_rust_caller_access",
     );
+    // caller_access calls itself without passing caller2_pubkey as meta account, not allowed
+    // without no_need_for_program_key_in_parent_instruction feature
     let account_metas = vec![
         AccountMeta::new_readonly(caller_pubkey, false),
         AccountMeta::new_readonly(caller2_pubkey, false),

runtime/src/bank/tests.rs

@@ -7523,7 +7523,7 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
             .unwrap()
     );
 
-    // Test not the system account
+    // Test not the rent account
     bank.clear_signatures();
     bank.store_account(&buffer_address, &buffer_account);
     bank.store_account(&program_keypair.pubkey(), &AccountSharedData::default());
@@ -7541,11 +7541,43 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
         .get_mut(1)
         .unwrap()
         .accounts
-        .get_mut(6)
+        .get_mut(4)
+        .unwrap() = AccountMeta::new_readonly(Pubkey::new_unique(), false);
+    let message = Message::new(&instructions, Some(&mint_keypair.pubkey()));
+    assert_eq!(
+        TransactionError::InstructionError(1, InstructionError::InvalidArgument),
+        bank_client
+            .send_and_confirm_message(
+                &[&mint_keypair, &program_keypair, &upgrade_authority_keypair],
+                message
+            )
+            .unwrap_err()
+            .unwrap()
+    );
+
+    // Test not the clock account
+    bank.clear_signatures();
+    bank.store_account(&buffer_address, &buffer_account);
+    bank.store_account(&program_keypair.pubkey(), &AccountSharedData::default());
+    bank.store_account(&programdata_address, &AccountSharedData::default());
+    let mut instructions = bpf_loader_upgradeable::deploy_with_max_program_len(
+        &mint_keypair.pubkey(),
+        &program_keypair.pubkey(),
+        &buffer_address,
+        &upgrade_authority_keypair.pubkey(),
+        min_program_balance,
+        elf.len(),
+    )
+    .unwrap();
+    *instructions
+        .get_mut(1)
+        .unwrap()
+        .accounts
+        .get_mut(5)
         .unwrap() = AccountMeta::new_readonly(Pubkey::new_unique(), false);
     let message = Message::new(&instructions, Some(&mint_keypair.pubkey()));
     assert_eq!(
-        TransactionError::InstructionError(1, InstructionError::MissingAccount),
+        TransactionError::InstructionError(1, InstructionError::InvalidArgument),
         bank_client
             .send_and_confirm_message(
                 &[&mint_keypair, &program_keypair, &upgrade_authority_keypair],
@@ -7555,6 +7587,35 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
             .unwrap()
     );
 
+    // Test not the system account
+    // NOTE: the system account is not required any more in the instruction accounts
+    bank.clear_signatures();
+    bank.store_account(&buffer_address, &buffer_account);
+    bank.store_account(&program_keypair.pubkey(), &AccountSharedData::default());
+    bank.store_account(&programdata_address, &AccountSharedData::default());
+    let mut instructions = bpf_loader_upgradeable::deploy_with_max_program_len(
+        &mint_keypair.pubkey(),
+        &program_keypair.pubkey(),
+        &buffer_address,
+        &upgrade_authority_keypair.pubkey(),
+        min_program_balance,
+        elf.len(),
+    )
+    .unwrap();
+    *instructions
+        .get_mut(1)
+        .unwrap()
+        .accounts
+        .get_mut(6)
+        .unwrap() = AccountMeta::new_readonly(Pubkey::new_unique(), false);
+    let message = Message::new(&instructions, Some(&mint_keypair.pubkey()));
+    assert!(bank_client
+        .send_and_confirm_message(
+            &[&mint_keypair, &program_keypair, &upgrade_authority_keypair],
+            message
+        )
+        .is_ok());
+
     fn truncate_data(account: &mut AccountSharedData, len: usize) {
         let mut data = account.data().to_vec();
         data.truncate(len);

sdk/program/src/message/compiled_keys.rs

@@ -37,7 +37,11 @@ struct CompiledKeyMeta {
 impl CompiledKeys {
     /// Compiles the pubkeys referenced by a list of instructions and organizes by
     /// signer/non-signer and writable/readonly.
-    pub(crate) fn compile(instructions: &[Instruction], payer: Option<Pubkey>) -> Self {
+    pub(crate) fn compile(
+        instructions: &[Instruction],
+        programs: &[Pubkey],
+        payer: Option<Pubkey>,
+    ) -> Self {
         let mut key_meta_map = BTreeMap::<Pubkey, CompiledKeyMeta>::new();
         for ix in instructions {
             let meta = key_meta_map.entry(ix.program_id).or_default();
@@ -53,6 +57,12 @@ impl CompiledKeys {
             meta.is_signer = true;
             meta.is_writable = true;
         }
+        for program in programs {
+            let meta = key_meta_map.entry(*program).or_default();
+            meta.is_signer = false;
+            meta.is_writable = false;
+            meta.is_invoked = true;
+        }
         Self {
             payer,
             key_meta_map,
@@ -241,6 +251,7 @@ mod tests {
                 Instruction::new_with_bincode(program_id2, &0, vec![]),
                 Instruction::new_with_bincode(program_id3, &0, vec![]),
             ],
+            &[],
             None,
         );
 
@@ -272,6 +283,7 @@ mod tests {
                 &0,
                 vec![AccountMeta::new_readonly(payer, false)],
             )],
+            &[],
             Some(payer),
         );
         assert_eq!(
@@ -296,6 +308,7 @@ mod tests {
                 &0,
                 vec![AccountMeta::new(id0, false), AccountMeta::new(id0, true)],
             )],
+            &[],
             None,
         );
 
@@ -325,6 +338,7 @@ mod tests {
                     AccountMeta::new(id0, true),
                 ],
             )],
+            &[],
             None,
         );
 
@@ -357,6 +371,7 @@ mod tests {
                 ),
                 Instruction::new_with_bincode(program_id, &0, vec![AccountMeta::new(id0, false)]),
             ],
+            &[],
             None,
         );
 

sdk/program/src/message/legacy.rs

@@ -311,7 +311,7 @@ impl Message {
         payer: Option<&Pubkey>,
         blockhash: &Hash,
     ) -> Self {
-        let compiled_keys = CompiledKeys::compile(instructions, payer.cloned());
+        let compiled_keys = CompiledKeys::compile(instructions, &[], payer.cloned());
         let (header, account_keys) = compiled_keys
             .try_into_message_components()
             .expect("overflow when compiling message keys");
@@ -326,6 +326,30 @@ impl Message {
         )
     }
 
+    /// Create a message with extra program ids
+    ///
+    /// Program ids for CPI are not always required in the instruction, so construct a message with program
+    /// ids in the message accounts.
+    pub fn new_with_programs(
+        instructions: &[Instruction],
+        programs: &[Pubkey],
+        payer: Option<&Pubkey>,
+    ) -> Self {
+        let compiled_keys = CompiledKeys::compile(instructions, programs, payer.cloned());
+        let (header, account_keys) = compiled_keys
+            .try_into_message_components()
+            .expect("overflow when compiling message keys");
+        let instructions = compile_instructions(instructions, &account_keys);
+        Self::new_with_compiled_instructions(
+            header.num_required_signatures,
+            header.num_readonly_signed_accounts,
+            header.num_readonly_unsigned_accounts,
+            account_keys,
+            Hash::default(),
+            instructions,
+        )
+    }
+
     /// Create a new message for a [nonced transaction].
     ///
     /// [nonced transaction]: https://docs.solana.com/implemented-proposals/durable-tx-nonces

sdk/program/src/message/versions/v0/mod.rs

@@ -257,7 +257,7 @@ impl Message {
         address_lookup_table_accounts: &[AddressLookupTableAccount],
         recent_blockhash: Hash,
     ) -> Result<Self, CompileError> {
-        let mut compiled_keys = CompiledKeys::compile(instructions, Some(*payer));
+        let mut compiled_keys = CompiledKeys::compile(instructions, &[], Some(*payer));
 
         let mut address_table_lookups = Vec::with_capacity(address_lookup_table_accounts.len());
         let mut loaded_addresses_list = Vec::with_capacity(address_lookup_table_accounts.len());