Skip to content

Commit

Permalink
Fix #21986 (#22035)
Browse files Browse the repository at this point in the history
* Partial revert "Updates documentation around what needs to be passed in CPI. (#21633)"

* Enforces the program_id being passed explicitly by removing it from get_instruction_keyed_accounts().

* instruction_accounts => instructions_account
  • Loading branch information
Lichtso authored Dec 21, 2021
1 parent 41ec7c8 commit ba8e158
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,12 @@ mod acme {

`invoke()` is built into Solana's runtime and is responsible for routing the
given instruction to the `token` program via the instruction's `program_id`
field. The caller has to pass all the accounts required by the instruction
being invoked, except for the executable account (with the key `program_id`).
field.

Note that `invoke` requires the caller to pass all the accounts required by the
instruction being invoked. This means that both the executable account (the
ones that matches the instruction's program id) and the accounts passed to the
instruction processor.

Before invoking `pay()`, the runtime must ensure that `acme` didn't modify any
accounts owned by `token`. It does this by applying the runtime's policy to the
Expand Down
21 changes: 11 additions & 10 deletions program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,15 @@ impl<'a> InvokeContext<'a> {

/// Get the owner of the currently executing program
pub fn get_loader(&self) -> Result<Pubkey, InstructionError> {
self.get_instruction_keyed_accounts()
.and_then(|keyed_accounts| keyed_accounts.first().ok_or(InstructionError::CallDepth))
.and_then(|keyed_account| keyed_account.owner())
let frame = self
.invoke_stack
.last()
.ok_or(InstructionError::CallDepth)?;
let first_instruction_account = frame
.number_of_program_accounts
.checked_sub(1)
.ok_or(InstructionError::CallDepth)?;
frame.keyed_accounts[first_instruction_account].owner()
}

/// Removes the first keyed account
Expand Down Expand Up @@ -759,18 +765,13 @@ impl<'a> InvokeContext<'a> {

/// Get the list of keyed accounts without the chain of program accounts
///
/// Note: The `KeyedAccount` at index `0` has the key `program_id` and
/// is followed by the `KeyedAccount`s passed by the caller.
/// Note: This only contains the `KeyedAccount`s passed by the caller.
pub fn get_instruction_keyed_accounts(&self) -> Result<&[KeyedAccount], InstructionError> {
let frame = self
.invoke_stack
.last()
.ok_or(InstructionError::CallDepth)?;
let first_instruction_account = frame
.number_of_program_accounts
.checked_sub(1)
.ok_or(InstructionError::CallDepth)?;
Ok(&frame.keyed_accounts[first_instruction_account..])
Ok(&frame.keyed_accounts[frame.number_of_program_accounts..])
}

/// Get this invocation's LogCollector
Expand Down
14 changes: 6 additions & 8 deletions programs/bpf/rust/instruction_introspection/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
extern crate solana_program;
use solana_program::{
account_info::next_account_info,
account_info::AccountInfo,
entrypoint,
entrypoint::ProgramResult,
Expand All @@ -25,20 +24,19 @@ fn process_instruction(
}

let secp_instruction_index = instruction_data[0];
let account_info_iter = &mut accounts.iter();
let instruction_accounts = next_account_info(account_info_iter)?;
assert_eq!(*instruction_accounts.key, instructions::id());
let data_len = instruction_accounts.try_borrow_data()?.len();
let instructions_account = accounts.last().ok_or(ProgramError::NotEnoughAccountKeys)?;
assert_eq!(*instructions_account.key, instructions::id());
let data_len = instructions_account.try_borrow_data()?.len();
if data_len < 2 {
return Err(ProgramError::InvalidAccountData);
}

let instruction = instructions::load_instruction_at_checked(
secp_instruction_index as usize,
instruction_accounts,
instructions_account,
)?;

let current_instruction = instructions::load_current_index_checked(instruction_accounts)?;
let current_instruction = instructions::load_current_index_checked(instructions_account)?;
let my_index = instruction_data[1] as u16;
assert_eq!(current_instruction, my_index);

Expand All @@ -56,7 +54,7 @@ fn process_instruction(
&[instruction_data[0], instruction_data[1], 1],
vec![AccountMeta::new_readonly(instructions::id(), false)],
),
&[instruction_accounts.clone()],
accounts,
)?;
}

Expand Down
5 changes: 4 additions & 1 deletion programs/bpf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1449,7 +1449,10 @@ fn test_program_bpf_instruction_introspection() {
);

// Passing transaction
let account_metas = vec![AccountMeta::new_readonly(sysvar::instructions::id(), false)];
let account_metas = vec![
AccountMeta::new(program_id, false),
AccountMeta::new(sysvar::instructions::id(), false),
];
let instruction0 = Instruction::new_with_bytes(program_id, &[0u8, 0u8], account_metas.clone());
let instruction1 = Instruction::new_with_bytes(program_id, &[0u8, 1u8], account_metas.clone());
let instruction2 = Instruction::new_with_bytes(program_id, &[0u8, 2u8], account_metas);
Expand Down
8 changes: 8 additions & 0 deletions sdk/program/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use crate::{
/// Notes:
/// - RefCell checking can be compute unit expensive, to avoid that expense use
/// `invoke_unchecked` instead, but at your own risk.
/// - The program id of the instruction being issued must also be included in
/// `account_infos`.
pub fn invoke(instruction: &Instruction, account_infos: &[AccountInfo]) -> ProgramResult {
invoke_signed(instruction, account_infos, &[])
}
Expand All @@ -17,6 +19,8 @@ pub fn invoke(instruction: &Instruction, account_infos: &[AccountInfo]) -> Progr
/// - The missing checks ensured that the invocation doesn't violate the borrow
/// rules of the `AccountInfo` fields that are wrapped in `RefCell`s. To
/// include the checks call `invoke` instead.
/// - The program id of the instruction being issued must also be included in
/// `account_infos`.
pub fn invoke_unchecked(instruction: &Instruction, account_infos: &[AccountInfo]) -> ProgramResult {
invoke_signed_unchecked(instruction, account_infos, &[])
}
Expand All @@ -26,6 +30,8 @@ pub fn invoke_unchecked(instruction: &Instruction, account_infos: &[AccountInfo]
/// Notes:
/// - RefCell checking can be compute unit expensive, to avoid that expense use
/// `invoke_signed_unchecked` instead, but at your own risk.
/// - The program id of the instruction being issued must also be included in
/// `account_infos`.
pub fn invoke_signed(
instruction: &Instruction,
account_infos: &[AccountInfo],
Expand Down Expand Up @@ -57,6 +63,8 @@ pub fn invoke_signed(
/// - The missing checks ensured that the invocation doesn't violate the borrow
/// rules of the `AccountInfo` fields that are wrapped in `RefCell`s. To
/// include the checks call `invoke_signed` instead.
/// - The program id of the instruction being issued must also be included in
/// `account_infos`.
pub fn invoke_signed_unchecked(
instruction: &Instruction,
account_infos: &[AccountInfo],
Expand Down

0 comments on commit ba8e158

Please sign in to comment.