From ec7536faf6397450942c8cb09b3122070a10df93 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Thu, 23 Dec 2021 17:43:15 -0600 Subject: [PATCH] Add test to enforce that program id account info for CPI is optional (#22069) * Update tests to demonstrate that program id account info for CPI is optional * Clean up comments that say that program id account info is required --- .../programming-model/calling-between-programs.md | 4 +--- programs/bpf/rust/instruction_introspection/src/lib.rs | 2 +- programs/bpf/rust/invoke_and_return/src/lib.rs | 5 +++-- programs/bpf/rust/upgradeable/src/lib.rs | 7 +++---- programs/bpf/rust/upgraded/src/lib.rs | 7 +++---- programs/bpf/tests/programs.rs | 9 ++------- sdk/program/src/program.rs | 8 -------- 7 files changed, 13 insertions(+), 29 deletions(-) diff --git a/docs/src/developing/programming-model/calling-between-programs.md b/docs/src/developing/programming-model/calling-between-programs.md index 96a41764b4..9de6b53099 100644 --- a/docs/src/developing/programming-model/calling-between-programs.md +++ b/docs/src/developing/programming-model/calling-between-programs.md @@ -57,9 +57,7 @@ given instruction to the `token` program via the instruction's `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. +instruction being invoked, except for the executable account (the `program_id`). 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 diff --git a/programs/bpf/rust/instruction_introspection/src/lib.rs b/programs/bpf/rust/instruction_introspection/src/lib.rs index 726ff265f8..b1ac55d03b 100644 --- a/programs/bpf/rust/instruction_introspection/src/lib.rs +++ b/programs/bpf/rust/instruction_introspection/src/lib.rs @@ -54,7 +54,7 @@ fn process_instruction( &[instruction_data[0], instruction_data[1], 1], vec![AccountMeta::new_readonly(instructions::id(), false)], ), - accounts, + &[instructions_account.clone()], )?; } diff --git a/programs/bpf/rust/invoke_and_return/src/lib.rs b/programs/bpf/rust/invoke_and_return/src/lib.rs index 9b4b81a6f4..88fc249eb3 100644 --- a/programs/bpf/rust/invoke_and_return/src/lib.rs +++ b/programs/bpf/rust/invoke_and_return/src/lib.rs @@ -14,7 +14,6 @@ fn process_instruction( instruction_data: &[u8], ) -> ProgramResult { let to_call = accounts[0].key; - let infos = accounts; let instruction = Instruction { accounts: accounts[1..] .iter() @@ -27,5 +26,7 @@ fn process_instruction( data: instruction_data.to_owned(), program_id: *to_call, }; - invoke(&instruction, infos) + // program id account is not required for invocations if the + // program id is not one of the instruction account metas. + invoke(&instruction, &accounts[1..]) } diff --git a/programs/bpf/rust/upgradeable/src/lib.rs b/programs/bpf/rust/upgradeable/src/lib.rs index 76a9f759b6..2186ab274a 100644 --- a/programs/bpf/rust/upgradeable/src/lib.rs +++ b/programs/bpf/rust/upgradeable/src/lib.rs @@ -8,13 +8,12 @@ use solana_program::{ entrypoint!(process_instruction); fn process_instruction( - program_id: &Pubkey, + _program_id: &Pubkey, accounts: &[AccountInfo], _instruction_data: &[u8], ) -> ProgramResult { msg!("Upgradeable program"); - assert_eq!(accounts.len(), 2); - assert_eq!(accounts[0].key, program_id); - assert_eq!(*accounts[1].key, clock::id()); + assert_eq!(accounts.len(), 1); + assert_eq!(*accounts[0].key, clock::id()); Err(42.into()) } diff --git a/programs/bpf/rust/upgraded/src/lib.rs b/programs/bpf/rust/upgraded/src/lib.rs index 5e5e63d0b0..152b58234f 100644 --- a/programs/bpf/rust/upgraded/src/lib.rs +++ b/programs/bpf/rust/upgraded/src/lib.rs @@ -8,13 +8,12 @@ use solana_program::{ entrypoint!(process_instruction); fn process_instruction( - program_id: &Pubkey, + _program_id: &Pubkey, accounts: &[AccountInfo], _instruction_data: &[u8], ) -> ProgramResult { msg!("Upgraded program"); - assert_eq!(accounts.len(), 2); - assert_eq!(accounts[0].key, program_id); - assert_eq!(*accounts[1].key, clock::id()); + assert_eq!(accounts.len(), 1); + assert_eq!(*accounts[0].key, clock::id()); Err(43.into()) } diff --git a/programs/bpf/tests/programs.rs b/programs/bpf/tests/programs.rs index 8222f33f47..a5773db269 100644 --- a/programs/bpf/tests/programs.rs +++ b/programs/bpf/tests/programs.rs @@ -1450,8 +1450,8 @@ fn test_program_bpf_instruction_introspection() { // Passing transaction let account_metas = vec![ - AccountMeta::new(program_id, false), - AccountMeta::new(sysvar::instructions::id(), false), + AccountMeta::new_readonly(program_id, false), + AccountMeta::new_readonly(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()); @@ -1743,7 +1743,6 @@ fn test_program_bpf_upgrade() { program_id, &[0], vec![ - AccountMeta::new(program_id.clone(), false), AccountMeta::new(clock::id(), false), ], ); @@ -1838,7 +1837,6 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() { program_id, &[0], vec![ - AccountMeta::new(program_id.clone(), false), AccountMeta::new(clock::id(), false), ], ); @@ -1925,7 +1923,6 @@ fn test_program_bpf_invoke_upgradeable_via_cpi() { invoke_and_return, &[0], vec![ - AccountMeta::new_readonly(program_id, false), AccountMeta::new_readonly(program_id, false), AccountMeta::new_readonly(clock::id(), false), ], @@ -2114,7 +2111,6 @@ fn test_program_bpf_upgrade_via_cpi() { invoke_and_return, &[0], vec![ - AccountMeta::new_readonly(program_id, false), AccountMeta::new_readonly(program_id, false), AccountMeta::new_readonly(clock::id(), false), ], @@ -2218,7 +2214,6 @@ fn test_program_bpf_upgrade_self_via_cpi() { program_id, &[0], vec![ - AccountMeta::new_readonly(noop_program_id, false), AccountMeta::new_readonly(noop_program_id, false), AccountMeta::new_readonly(clock::id(), false), ], diff --git a/sdk/program/src/program.rs b/sdk/program/src/program.rs index 45873e2d64..f07099a0f5 100644 --- a/sdk/program/src/program.rs +++ b/sdk/program/src/program.rs @@ -7,8 +7,6 @@ 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, &[]) } @@ -19,8 +17,6 @@ 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, &[]) } @@ -30,8 +26,6 @@ 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], @@ -63,8 +57,6 @@ 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],