Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature - Lift CPI Caller Restriction #2202

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 31 additions & 20 deletions program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ use {
},
solana_compute_budget::compute_budget::ComputeBudget,
solana_feature_set::{
move_precompile_verification_to_svm, remove_accounts_executable_flag_checks, FeatureSet,
lift_cpi_caller_restriction, move_precompile_verification_to_svm,
remove_accounts_executable_flag_checks, FeatureSet,
},
solana_log_collector::{ic_msg, LogCollector},
solana_measure::measure::Measure,
Expand Down Expand Up @@ -429,28 +430,38 @@ 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(|| {
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)?;
#[allow(deprecated)]
if !self
let program_account_index = if self
.get_feature_set()
.is_active(&remove_accounts_executable_flag_checks::id())
&& !borrowed_program_account.is_executable()
.is_active(&lift_cpi_caller_restriction::id())
{
ic_msg!(self, "Account {} is not executable", callee_program_id);
return Err(InstructionError::AccountNotExecutable);
}
self.transaction_context
.find_index_of_program_account(&callee_program_id)
.ok_or_else(|| {
ic_msg!(self, "Unknown program {}", callee_program_id);
InstructionError::MissingAccount
})?
} else {
let program_account_index = instruction_context
.find_index_of_instruction_account(self.transaction_context, &callee_program_id)
.ok_or_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)?;
#[allow(deprecated)]
if !self
.get_feature_set()
.is_active(&remove_accounts_executable_flag_checks::id())
&& !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()
};

Ok((
instruction_accounts,
vec![borrowed_program_account.get_index_in_transaction()],
))
Ok((instruction_accounts, vec![program_account_index]))
}

/// Processes an instruction and returns how many compute units were used
Expand Down
5 changes: 1 addition & 4 deletions programs/sbf/tests/programs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1203,10 +1203,7 @@ fn test_program_sbf_caller_has_access_to_cpi_program() {
];
let instruction = Instruction::new_with_bytes(caller_pubkey, &[1], 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)
);
assert!(result.is_ok());
}

#[test]
Expand Down
16 changes: 6 additions & 10 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7746,16 +7746,12 @@ fn test_bpf_loader_upgradeable_deploy_with_max_len() {
.get_mut(6)
.unwrap() = AccountMeta::new_readonly(Pubkey::new_unique(), false);
let message = Message::new(&instructions, Some(&mint_keypair.pubkey()));
assert_eq!(
TransactionError::InstructionError(1, InstructionError::MissingAccount),
bank_client
.send_and_confirm_message(
&[&mint_keypair, &program_keypair, &upgrade_authority_keypair],
message
)
.unwrap_err()
.unwrap()
);
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();
Expand Down
5 changes: 5 additions & 0 deletions sdk/feature-set/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,10 @@ pub mod remove_accounts_executable_flag_checks {
solana_pubkey::declare_id!("FfgtauHUWKeXTzjXkua9Px4tNGBFHKZ9WaigM5VbbzFx");
}

pub mod lift_cpi_caller_restriction {
solana_pubkey::declare_id!("HcW8ZjBezYYgvcbxNJwqv1t484Y2556qJsfNDWvJGZRH");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -1086,6 +1090,7 @@ lazy_static! {
(disable_sbpf_v1_execution::id(), "Disables execution of SBPFv1 programs"),
(reenable_sbpf_v1_execution::id(), "Re-enables execution of SBPFv1 programs"),
(remove_accounts_executable_flag_checks::id(), "Remove checks of accounts is_executable flag SIMD-0162"),
(lift_cpi_caller_restriction::id(), "Lift the restriction in CPI that the caller must have the callee as an instruction account #2202"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down
Loading